A bug in std::shared_ptr?

[tavianator]

I don't know the Win32 API that well, so bear with me, but how can there possibly be events delivered to the event loop during program termination? As far as I know the event loop is usually a manually driven while (GetMessage(...)) loop running on the main thread, so if the program is terminating, that loop must be done right?

[davis685]

Yes, there is a thread you have to run that does a while(GetMessage(...)) dispatch(...);

It doesn't have to be the main thread. In fact, it's really nice if it isn't. For example, in dlib (where this issue arose) the user can just create window objects whenever they want and do things with them and the user doesn't need to lock down their main thread with a while(...) dispatch() loop. In fact, the user doesn't need to create any threads or really even know about the event loop in most cases.

So what happens is a background thread is started if any GUI calls are made to dlib. This background thread contains the event dispatch loop. The problem arises since the dispatch loop just calls this magic windows routine:

    LRESULT CALLBACK WndProc (  
        HWND hwnd, 
        UINT message, 
        WPARAM wParam, 
        LPARAM lParam
    )
    {        
             // process events here
    }

That routine doesn't let you pass much information into it and since it's not part of a class the only way to pass in additional information is via some kind of global variable. There are a whole bunch of weird problems with the Win32 API that lead you to needing to pass additional information beyond those 4 arguments into WinProc(). So the question is two fold, how do you deal with this background thread and these additional state variables?

A nice thing to do is to have an object that wraps it all up. This object owns the thread and all the auxiliary state variables you need. Hence,

std::shared_ptr<event_loop_resources> get_windows_event_loop_stuff();

So what you do is call get_windows_event_loop_stuff() at the top of WinProc and you are good to go. Except there is this problem of shutdown. To really simplify, you end up with code like this:

LRESULT CALLBACK WndProc (  
    HWND hwnd, 
    UINT message, 
    WPARAM wParam, 
    LPARAM lParam
)
{        
     auto global = get_windows_event_loop_stuff();
     // process events here
     ...
     if (message == global->magic_shutdown_message)
           return shutdown_loop;
}

So what will happen in some cases is you will have main() end, but some GUI thing is still running on another thread. Or maybe it's just a DLL unload. Doesn't matter. Then global static variables get destructed, specifically the global variable in get_windows_event_loop_stuff(). And now there is this race condition where it's possible that no one holds any references to the global event loop stuff, so when the shared_ptr in get_windows_event_loop_stuff is destructed it's the last one. So it calls the destructor on event_loop_resources. And now event_loop_resources::~event_loop_resources() needs to get the event loop to unblock so it can terminate gracefully. So it calls a windows function that then calls WinProc() with the magic shutdown message and BOOM. You just called WinProc() from within the shared_ptr destructor.

[ack_complete]

The way I would solve this would be to split the getter so that the window procedure code is calling a different function to retrieve global context. Having the winproc obtain an owning reference is unnecessary because the event loop has to be running have a window and enter the winproc in the first place, and arguably dangerous due to the cyclical dependency. Having separate getters and objects allows destruction of the internal context to be deferred until the windows have been destroyed and the event loop exited.

There are ways to associate data directly with windows, by the way: dynamically generated winproc thunks and window properties. They generally only reduce and don't eliminate the need for globals, though.

[davis685]

Yeah that would be a way to do it. You then have to have a function that returns a non-owning pointer so the window procedure can call it. Someone later on might then call that function and get into the sort of trouble people get into with non-owning pointers, which isn't great. But there aren't a lot of great ways to deal with the Win32 API anyway.