7

I know that the invalid value returned by CreateFile is INVALID_HANDLE_VALUE. But since I also like to use RAII it's very tempting to just stick the HANDLE in a shared_ptr (like this: shared_ptr<void> handle (CreateFile(args),&CloseHandle)) to make sure that the handle is closed. My only concern with this quick and easy way to do RAII is if CreateFile can return NULL as the HANDLE value.

Martin B
  • 23,670
  • 6
  • 53
  • 72
Dan
  • 1,981
  • 1
  • 14
  • 18
  • 2
    Note that you have to be careful when trying to do something like this. See: http://stackoverflow.com/questions/1562421/making-a-handle-raii-compliant-using-shared-ptr-with-a-custom-deleter – tenfour Nov 23 '11 at 11:08
  • 1
    It's ok to create a RAII wrapper for this kind of resource. However don't use `shared_ptr` unless you **really** need it to be shared. It's a huge overkill (`shared_ptr` internally allocates an extra memory on the heap for reference counting), plus you'll not have a convenient access, such as a direct casting to `HANDLE`, because `shared_ptr` deliberately prevents this. – valdo Nov 23 '11 at 11:50
  • excellent points. unique_ptr would probably be a better way to go .. at this level of effort. – Dan Nov 23 '11 at 13:37
  • What's the issue anyway? `CloseHandle` works just fine in presence of null (not quite as silently as `delete`, but still). `GetLastError` will return 0x06 afterwards, and if you're running in the debugger, it will break on the exception, but who cares. After all _it is_ an error, so that's good behaviour. It's not like `CloseHandle(0)` means the end of the world. – Damon Nov 23 '11 at 17:37

3 Answers3

8

NULL is not a valid handle value. You can discern this from the fact that some Windows API functions return NULL to indicate a failure. Since there is a single function to dispose of handles, CloseHandle, it follows that NULL is not a valid HANDLE value. Hence CreateFile cannot ever return NULL.

Raymond Chen wrote a blog article touching on this topic: Why are HANDLE return values so inconsistent?.

Now, I know nothing about shared_ptr<> so would like to make no comment on whether or not your idea is appropriate. I am merely answering the direct question that you asked.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • I was not aware that `NULL` may not be returned even from functions for which "invalid" return value is `INVALID_HANDLE_VALUE`. Are you sure this is so? Your argument is that by such `CloseHandle` may detect when invalid handle is passed to it, no matter which function returned it (as long as it's a kernel handle). But OTOH `CloseHandle` does not **have** to carry about invalid values passed to it. – valdo Nov 23 '11 at 11:56
  • 2
    @valdo My argument is that `CloseHandle` doesn't know whether or not the invalid handle came from CreateFile or another API that uses `NULL` as its return value. Another simpler argument was that if `CreateFile` returned `NULL`, how could another function signal failure? – David Heffernan Nov 23 '11 at 12:00
  • 1
    Alright, I understood your argument, and it's probably correct. However *theoretically* this does not prove anything. According to the standard `CloseHandle` does not have to realize that you passed an invalid handle to it and just silently fail without doing any harm, such as closing another (unrelated) resource. It's your problem. It's your responsibility to check the return value of `CreateXXXX` and not call `CloseHandle` if the handle is invalid. However, practically speaking, I believe both `NULL` and `INVALID_HANDLE_VALUE` are reserved as a non-handle values. – valdo Nov 23 '11 at 15:11
  • 1
    Doesn't make sense, really. If we presume that `NULL` is a valid _file_ handle, then there would be no problem if `CloseHandle` closed that file. Sure, `CreateMutex` may return `NULL` if it couldn't create a mutex. But you may not call `CloseHandle` after that, because `CloseHandle` only closes handles to objects. And if CreateMutexed failed, the returned `NULL` is **not** an object handle but an error value. IOW, don't pass invalid handles to `CloseHandle`, that violates the explicit precondition "A valid handle to an open object." – MSalters Nov 23 '11 at 15:53
  • 1
    @MSalters Just think of the fun that would ensure if `NULL` was a valid file handle. Then every programmer that made the mistake of calling `CloseHandle` without checking for errors would pay a very heavy price. What's more, if `NULL` were to be a valid value then the system would have to maintain two per-process lists of open handles rather than just one. I agree that I have proved nothing 100%. – David Heffernan Nov 23 '11 at 15:55
  • @David: It would have been rather obvious rather quickly, I'd say. But yes, as a reader of Raymond Chen's blog, I know that AppCompat will ensure that such ivalid programs will continue to run. – MSalters Nov 23 '11 at 16:02
  • @DavidHeffernan: `msdn:CloseHandle` does not explicitely document that CloseHandle(0) is a no-op, and calling `CloseHandle(0)` "out of the blue" returns false and sets lasterror to `INVALID_HANDLE_VALUE`. I agree with "MS won't do that, think of the chaos that would ensue", but legally, API-wise, from the MSDN documentation, 0 is a valid handle value. Unfortunately. – peterchen Oct 26 '15 at 17:23
  • @peterchen If 0 was a valid handle value, how could it also be a sentinel handle value? – David Heffernan Oct 26 '15 at 17:34
  • @DavidHeffernan: 0 is a valid *file* handle value, but not a valid handle for e.g. a Mutex. At least, there's nothing in the documentation that prohibits this use, and I can't see what would prohibit an implementation to do so. I.E. the caller would need to track whether it's a file or a non-file handle, and use the appropriate sentinel. --- now, I'm certain my code would break if there ever was a file handle 0. – peterchen Oct 27 '15 at 09:50
  • @peterchen I don't agree that 0 is a valid file handle. – David Heffernan Oct 27 '15 at 09:59
  • @DavidHeffernan: In practice? We all hope not. But **nothing** in the `CreateFile`documentation says it can't. The sentinel for that function is `(HANDLE)(LONG_PTR)-1`. Raymond Chen's post you linked doesn't say anything else. – peterchen Oct 27 '15 at 10:07
  • @peterchen I don't think the documentation is required to list all invalid values – David Heffernan Oct 27 '15 at 10:10
  • @DavidHeffernan: No, it's not. But similary, we *are* required to not assume any unlisted values was, is, and always will be invalid. No mater how tempting. – peterchen Oct 27 '15 at 10:35
  • @peterchen I wasn't assuming. My argument is that the invalid handle values are at least the union of all sentinels used by handle creation routines. – David Heffernan Oct 27 '15 at 10:43
  • @DavidHeffernan: I do think this assumption is formally incorrect, since functions reject sentinel values as invalid handle. All functions that do accept file handles could handle 0 just fine. It would be incorrect to call them with 0 otherwise. E.g. `SetEvent` would always return `INVALID_HANDLE_VALUE` because it can only be called with valid event handles which are never 0. `WaitForSingleObject` can wait for a file handle, so if a file wiht handle 0 were open it would wait for that file handle. – peterchen Oct 28 '15 at 11:18
  • @peterchen I think we are going to have to agree to disagree here – David Heffernan Oct 28 '15 at 12:14
1

When testing a HANDLE for validity in a generic way, check for both NULL and INVALID_HANDLE_VALUE.

But I don't see how RAII has anything to do with whether CreateFile can return NULL. You will need to provide custom code for testing validity and deallocating in order to make HANDLE work with a shared pointer, so you are in control of these checks, not the shared pointer class.

In other words, it makes no difference whether it's in a shared pointer or you use a normal HANDLE, the checks are exactly the same, and you must provide them either way.

tenfour
  • 36,141
  • 15
  • 83
  • 142
  • 1
    It's actually rather benign to call `CloseHandle` passing `NULL` or `INVALID_HANDLE_VALUE`. It will be an error, but no harm will come of this. I admit that I would not wrap up a `HANDLE` this way. A bespoke `RAII` class would get the job done well. I'm sure that's been done a gazillion times. +1 – David Heffernan Nov 23 '11 at 11:52
  • @David Heffernan I'm well aware that the more complete solution would be to write your own handle management class. Lets consider this a step in that direction. – Dan Nov 23 '11 at 13:12
  • as for why I care if CreateFile can return NULL and what that has to do with RAII. I was concerned that shared_ptr might not call the custom destructor if the value was null.. upon looking deeply at the code ( at least for boost) it looks like it does anyway.. Also if using custom creation functions that return a shared_ptr distinguishing between a shared_ptr that contains nothing vs one that contained a null handle would be troublesome – Dan Nov 23 '11 at 13:32
0

CreateFile never returns NULL. I suggest you to use already created wrapper ATL::CAtlFile and don't envent a new one based on shared_ptr.

Sergey Podobry
  • 7,101
  • 1
  • 41
  • 51
  • that's all well and good, but lets say that I actually wanted to add in testing code in my custom destructor before actually closing the handle. – Dan Nov 23 '11 at 13:10
  • @Dan: Well, then you can inherit CAtlFile and do you testing code in destructor. IMHO shader_ptr for handle looks ugly. – Sergey Podobry Nov 23 '11 at 16:48
  • It has handy WinApi wrappers for synchronization, security, file mapping, window and etc. I agree it is not perfectly designed but it is out-of-the-box that is good. – Sergey Podobry Aug 08 '13 at 18:36