Proper way close WinAPI HANDLEs (avoiding of repeated closing)

Question

I have some handle and I need to close it. There is some places in code, where handle may be closed. So, is this a right way to close handle?

HANDLE h;
....
if ( h != INVALID_HANDLE_VALUE ) {
  ::CloseHandle(h);
  h = INVALID_HANDLE_VALUE;
}

There is a same question about bitmap handles:

HBITMAP hb;
....
if ( hb != INVALID_HANDLE_VALUE ) {
  ::DeleteObject(hb);
  hb = INVALID_HANDLE_VALUE;
}

EDIT: I think, there is some misunderstanding. I know CloseHandle is for closing handles. I'd like to know proper way for closing handles. A similar situations occurs with deleting of pointers.

Foo *foo = new Foo();

// for example there is 2 functions that can delete foo
void bar() {
  ....
  delete foo;
}
void duck() {
  ....
  delete foo;
}

So, the following code means problems:

bar();
duck();

There is some workaround for this case. We need to define bar&duck functions like this:

void bar() {
  ....
  if (foo) {
    delete foo;
    foo = NULL;
  }
}
void duck() {
  ....
  if (foo) {
    delete foo;
    foo = NULL;
  }
}

So we avoid repeated deleting of foo. The question is What is the proper way to close handles? I mean, How to avoid repeated closing handles problem?

Plight

The answer may surprise you, but yes, if the documentation does not state otherwise, then CloseHandle is indeed the correct function to call if you want to close a handle. But there are some exceptions, some functions create handles which should be closed differently. So either show us how the handle was created, or look it up yourself on MSDN :) – Lacedaemon

The point is that kernel handles are closed with CloseHandle, while other handles (usually GDI handles) have their own function. Still, on the MSDN page about the function used to acquire the handle is always listed the corresponding function to call to release it. – Ascomycete

The check and setting for INVALID_HANDLE_VALUE may well be pointless if the handle can be closed from multiple threads - you need to be prepared for CloseHandle() to fail anyway. – Tomasine

@MartinJames: If a handle can be closed from multiple threads without synchronization you've got much worse problems than CloseHandle failing. The handle will be reused by the kernel and you'll then close something you didn't intend to close. This problem can be very hard to track down because it never reproduces the same way twice. – Beavers

Answer 1

Not all functions that use HANDLE use CloseHandle(), some use other closing functions instead. Also, not all HANDLE values use INVALID_HANDLE_VALUE, either. Some use NULL instead.

HBITMAP never uses INVALID_HANDLE_VALUE, it always uses NULL. And you should never call DeleteObject() for an HBITMAP you do not own.

So the short answer is - if you are trying to create some general purpose handle management, don't bother. You are likely to get it wrong. If you allocate/open some handle, you have to know the correct way to close it, you can't guess at it.

If you want the handles to manage themselves, then RAII is the best choice. I prefer to use a templated class with specialized traits to reduce code duplication for differrent types of handles, eg:

template< class traits >
class HandleWrapper
{
private:
    traits::HandleType FHandle;

public:
    HandleWrapper()
        FHandle(traits::InvalidValue)
    {
    }

    HandleWrapper(const traits::HandleType value)
        FHandle(value)
    {
    }

    ~HandleWrapper()
    {
        Close();
    }

    void Close()
    {
        if (FHandle != traits::InvalidValue)
        {
            traits::Close(FHandle);
            FHandle = traits::InvalidValue;
        }
    }

    bool operator !() const {
        return (FHandle == traits:::InvalidValue);
    }

    operator bool() const {
        return (FHandle != traits:::InvalidValue);
    }

    operator traits::HandleType() {
        return FHandle;
    }
};

.

struct KernelHandleTraits
{
    typedef HANDLE HandleType;
    static const HANDLE InvalidValue = INVALID_HANDLE_VALUE;

    static void Close(HANDLE value)
    {
        CloseHandle(value);
    }
};

HandleWrapper<KernelHandleTraits> hFile(CreateFile(...));

.

struct NullKernelHandleTraits
{
    typedef HANDLE HandleType;
    static const HANDLE InvalidValue = NULL;

    static void Close(HANDLE value)
    {
        CloseHandle(value);
    }
};

HandleWrapper<NullKernelHandleTraits> hMapping(CreateFileMapping(...));

.

struct FileMapViewTraits
{    
    typedef void* HandleType;
    static const void* InvalidValue = NULL;

    static void Close(void *value)
    {
        UnmapViewOfFile(value);
    }
};

HandleWrapper<FileMapViewTraits> hView(MapViewOfFile(...));

.

struct GDIBitmapHandleTraits
{    
    typedef HBITMAP HandleType;
    static const HBITMAP InvalidValue = NULL;

    static void Close(HBITMAP value)
    {
        DeleteObject(value);
    }
};

HandleWrapper<GDIBitmapTraits> hBmp(CreateBitmap(...));

Etc.

Bahamas

I like this code (+1). BTW, it isn't RAII, because Acquiring resource(s) in constructor is not mandatory in RAII idiom but releasing resources in the destructor is the key. And your mechanism is for manually releasing resources. – Plight

The destructor does release the resource, if it is still acquired. I include support for manual releasing because sometimes waiting for the destructor can be too late, sometimes on-demand releasing is desired. – Bahamas

Also, not all HANDLEs are handles ;) GetCurrentProcess (...) is a no-op on all versions of Windows that returns -1. You cannot close that handle, it is meaningless. – Feria

True, but in that case, you wouldn't need to use this wrapper for that handle to begin with. Use this only for handles that need to be explicitly closed. – Bahamas

I'd label this RRID (resource relase is destruction) rather than RAII. With RAII, I'd expect constructors that acquire the resource (in addition to the destructor that releases it). I know not everyone draws that distinction, but I find it useful to distinguish between strict RAII and transferring a resource to a smartpointer-like owner. – Ansermet

Why are copy/move constructors/assignment operators not implemented in accordance with a type of ownership? – Sup

@oleksijp at the time this answer was written, move semantics were still fairly new and not widely used yet. And uses of such classes are usually isolated in scope so copies are rare, and copy semantics depend on the type of handle used and which API manages the handle. It is certainly possible to add copy/move semantics with some work, they just weren't needed at the time. – Bahamas

Answer 2

Use RAII pattern.

Wrap a handle into a class which allocates the handle in the constructor and destroys it in the destructor. You can find some examples in MFC, e.g. CGdiObject class for GDI objects like HBITMAP.

See also this SO question: RAII and smart pointers in C++

Barbellate

I cannot use RAII, because I need to close handle before the hosting class destroyed, sometimes. But my question isn't about language metaphors. – Plight

@Loom: If you can't use RAII, there must be a problem with architecture of your program, as the example with pointers shows. – Barbellate

The code with pointers is just illustrate problem. Actually, my question isn't about architecture, too. Anyway, thank you for your opinion. – Plight

RAII can still be used, simply add a method to the class that closes the current handle on demand, but still let the destructor also close the handle if it has not already been done. – Bahamas

@Loom: Your "hosting" class is not the class which should call CloseHandle. Instead, you add a new class Win32Handle, and your hosting class will now contain one or more Win32Handles. This setup is directly related to your question: It's the Win32Handle class which prevents duplicate closing. This puts the responsibility in one place, instead of scattering CloseHandle calls all over your code. – Ribbing

Answer 3

Yes.

• CloseHandle() closes windows kernel object handles.
• DeleteObject() deletes GDI objects.

I think your confusion comes from them both being called "handles", but they are different "classes" of objects. The term handle in HBITMAP is used here more as "opaque identifier". There is also plenty of documentation which assumes "handle" == "windows kernel handle".

In general if you're wondering how to delete something you should look at the constructor's documentation.

Endometriosis

Answer 4

The following code is maybe what you're after:

BOOL CloseValidHandle(HANDLE& handle)
{
  if (handle != INVALID_HANDLE_VALUE && handle != 0)
  {
    if (CloseHandle(handle))
    {
      handle = INVALID_HANDLE_VALUE;
      return TRUE;
    }
    else
    {
      return FALSE;
    }
  }

  return TRUE;
}

Fog

Why all that checking? Either CloseHandle() succeeds, or it does not. – Tomasine

And what are you going to do with the return value from your function? Cook it for supper? – Hemianopsia

Use it or don't use it. I use it. – Fog

Handle parameter is a reference to HANDLE. If handle is successfully closed then that handle becomes INVALID_HANDLE_VALUE, and second call is no-op. The actual problem is if you have two variables with the same handle value and try to close both of them. Such situation should be avoided. – Fog

Answer 5

It's no RAII but it helps to delete/close handler.

class HandleDel : boost::notcopyable{
public:
    HandleDel(HANDLE h, HANDLE invalid, BOOL(WINAPI *del)(HANDLE)):
        h(h), invalid(invalid), del(del){
    }
    ~HandleDel(){
        if ( h != invalid ) del(h);
    }
private:
    HANDLE h;
    HANDLE invalid;
    BOOL(WINAPI *del)(HANDLE);
};

Oldenburg

Why do you say this is not RAII? It is. – Bahamas