Reputation: 26281
I was inspecting someone's code and I saw this:
template<typename Data>
class ConcurrentQueue {
private:
HANDLE dataPushEvent;
// More Private Members...
public:
ConcurrentQueue() {
dataPushEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
}
// Public Methods...
};
As you can see, there is no destructor on this class, and the dataPushEvent
is not explicitly freed anywhere in this class. Since it's a private member, it cannot be accesed externally, so I was thinking this'd probably create a memory leak.
Unless this handle is not supposed to be disposed.
I'm new to C++ and Windows programming. As far as I'm concerned, HANDLE
is a void *
, and as all pointers, its reference should be released when we are done using it.
Am I right?
Upvotes: 0
Views: 2037
Reputation: 169008
If there is no destructor, then yes, the handle is leaked. There should be a destructor calling CloseHandle()
to destroy the event.
As a side note, the class should also delete (C++11) or make inaccessible (C++03) the copy constructor and copy assignment operators, since the compiler-generated defaults will not make sense -- they are likely to copy the queued data, but still reference the same event handle, causing multiple seemingly-independent queues to share an event for no good reason. And when you do implement the destructor, the default copy implementation will be even worse because once one copy gets destructed it will destroy the event handle still in use by all of the other copies!
In C++11, you can implement the move-constructor and the move-assignment operators, but you'll have to tweak your destructor to account for the fact that the handle might have been moved to another object so that it doesn't call CloseHandle()
.
Upvotes: 1