Proper multithreaded clean-up

Started by
2 comments, last by irreversible 9 years ago

It has been one of those daunting chores I've been postponing for quite a while. In particular, this thread is a spiritual follow-up to my multithreaded GUI discussion from almost a year ago.

While there is at least one instance I know of where my code sometimes locks up after I initiate clean-up, I've managed to avoid more glaring conflicts for the most part by taking special care of how I manage my GUI code. More precisely, I've avoided situations where I need to destroy and create windows at run-time. I'm now finally looking into implementing some fashion of robust clean-up of GUI elements, but I'm having trouble properly outlining the problem.

The basic element of my GUI system is a window. A window can be created from any thread and after it is instantiated its respective parts are handled in their respective threads. Multithreaded changes/queries of core properties are handled via atomics. Since not all variables in my window class are thread-safe, the entire architecture currently relies on a certain level of logic and caution. At the end of the day this has, however, proven to be quite natural and hasn't really made me give it a second thought.

Destroying a window is a lot more complicated, though, and is further compounded by the hierarchy the window belongs to. While I can assume trivial cases to work without a hitch pretty much 100% of the time, problems arise when various threads are allowed to duplicate a window's handle and the window itself is anything more complicated than a message box with two buttons.

Right now my memory manager implements a very crude pseudo smart pointer layer that doesn't even do reference counting (though I guess it's not that smart then). The smart pointer rather performs as an interim data structure that stores information about the object in the manager's memory pool. All GUI classes are object-oriented, depend on polymorphism and are not wrapped by an indirect access layer the way WinAPI does it. This further complicates things.

So, to postulate the problem: I'm having trouble determining when and how it is safe to free the memory that belongs to a window. This concern owes to several potential caveats:

1) the window could still be handling a mouse message (which is easy to determine)

2) it could be handling a keyboard message (also trivial)

3) it could be performing drawing (ditto)

4) one of its exposed functions could be accessed (annoying or impossible to determine since there are many member functions and derived classes cannot be relied on to guard their accessors)

5) one of the threads in 1-3 could have spawned a task that will access the window's handle duplicate from a worker thread at a later time (which is impossible to determine)

I don't know how precisely Windows does this, but the fact that it wraps access to a generic handle certainly seems to alleviate a lot of the headaches related to point 5.

Since I do not wish to use a similar layer of abstraction, I want to figure out a way to guarantee that the app won't crash when it inadvertently destroys a window and then later tries to access it; or rather - I want to provide the programmer (err, in this case myself) at least some information to let them know they've screwed up. In particular, I'd be happy if I could trap access to a destroyed window and in the very least provide information on where the access occurs, where it was destroyed and which dead window is being accessed.

The only way I can currently think of achieving this would be to:

1) provide template specialization for window handle duplication, which stores all new pointers/handles and modifies them when destruction occurs. More precisely, I would disable direct assignment of window pointers and have the programmer marshal assignment through a DuplicateWindowHandle() call

2) when a window is destroyed, the stored list of handles is modified to point to a dummy window. All subsequent calls to this handle would throw, notifying that the program is accessing a dead handle, where the access comes from and what the dead window is that is being accessed. I already have almost all of this info. I assume I've already missed something in my logic, but even if I haven't, there's a hitch...*/**

This is where my knowledge of multithreading fails me and I would appreciate some insight and ideas. The following paragraph is admittedly mostly nonsense and entwined with a serious lack of sleep and crude over-thinking.

* This is the source of my headache: I still can't figure out when it's safe to actually destroy the window. Notably, I'm not sure how to guarantee no access to the window during the destruction/redirection process while the handles are being redirected. In the very least I want to avoid a hard crash. The only way I think I can (at least) partially handle this is by adding a critical section around the smart pointer's cast operator. That, however, would add a critical section to each and every pointer. Which sounds just plain stupid. That is, unless I roll a separate pointer class for the window code, which - even if this worked - sounds far more dubious than I feel comfortable with. In any case my logic fails me. Help.

** A further impact of this would be that I wouldn't know how to handle access to a derived window class, which introduces new variables and/or non-standard accessor functions.

Sigh...

Advertisement

Hm. Can you just shoehorn in thread safe checks with branching, or is the code too intense/critical?

For example, can you create/destroy windows accurately (after all processing is completed), while on certain screens?

In this example, if the user opens "advanced video" settings, can you destroy the "video" window?

If the user closes the advanced video window, can you destroy the video window?

Start

| |

Video Audio

| |

Advanced Sound test

If both of those are true, I'd suggest simply adding a variable to each window object to track if it has an operation running. Just make it a byte/int, and incriment 1 at the start of a new threaded request, decrease by 1 at the end of a threaded request. Only delete windows if that counter is 0.

Instead of trying to make it thread safe, just cheese your way around it. Way less headaches for better productivity. Trying to get a GUI perfect will be painful, so doing things simpler will probably help a ton.

You don't have a threading problem - you have an ownership problem.

Something needs to own the window. That thing will be responsible for cleaning it up. No one else should ever clean it up. If you are able to do this, then use a unique_ptr-like object. It cannot be copied, it can only be moved, and it deletes the object it contains in its destructor. No one else is allowed to store pointers to the object anywhere (you can pass them as function parameters, under the understanding that the caller will not let the object go away while the function executes).

If you absolutely must share ownership, or store the pointers somewhere else, then you should use a shared_ptr/weak_ptr system where you reference count the object and delete it when all non-weak references are gone. If someone tries to promote a weak reference to a strong one and the object is deleted, they will get a null pointer.

Assuming you're actually referring to windows in your own GUI system that are new-ed classes and not Windows' HWNDs, and are using a C++11/14 compliant compiler, then you can just use the unique_ptr/shared_ptr/weak_ptr directly rather then making your own.

You don't have a threading problem - you have an ownership problem.

This has probably been one of the more helpful self-contained sentences anyone's told me in a long time.

I was so focused on solving the problem at hand and looking at it from so up close that I never considered that I might be looking at it from a completely wrong direction. I've since mentally remapped the logic of my entire code base and while rewriting crucial parts of it are frankly a pain, this simple shift in mentality cleared up a lot of the confusion I was having.

This topic is closed to new replies.

Advertisement