Proposed patch to avoid leaking a HANDLE in threading code


#1

Hi,

during debugging, Microsoft Application Verifier has found a leaking HANDLE in the code.

Tracking the issue down, I found that the leaking handle is the one created in juce_win32_Threads.cpp, at this point:

struct SleepEvent
{
    SleepEvent()
        : handle (CreateEvent (0, 0, 0,
                    #if JUCE_DEBUG
                       _T("Juce Sleep Event")))
                    #else
                       0))
                    #endif
    {
    }

    HANDLE handle;
};

As you can see, the handle is created with CreateEvent, but never destroyed. I added matching code in the destructor, as follows:

~SleepEvent ()
{
    CloseHandle (handle);
}

and this was sufficient to get rid of the leaking handle warning.


#2

TBH that’s a deliberate leak, because there’s a risk that somebody could write a static destructor that performs a sleep, which could crash if that handle has already been destroyed.


#3

hmm, what about handling it like a fully implemented Singleton then?


#4

That seems like overkill for the sake of one handle. The thing is that it’s not really a “leak”, because the handle is required for the lifetime of the app, and the OS cleans it up afterwards, so everything works perfectly well the way it’s written.

I guess I could add some debug-only code that releases the handle, just to avoid the warning…


#5

I have been reported of the leak even with the Release build though… isn’t there any other way? For example, doing the other way around, and jasserting the object to be initialised when used?


#6

… ok, I’ll figure something out.


#7

wait wait, I think there is a better way to handle this: what about binding the existence of that handle wrapper to the existence of the Thread class itself, for example making it a static member of the class itself? This way, we should be sure that the object doesn’t get destroyed until there are no more calls to Thread::sleep to be expected (am I right?)


#8

No, Thread::sleep is a static method. But I’ve already checked something in that should sort it out with little extra overhead.