Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Cleaning up threads in a DLL: _endthreadex() vs TerminateThread()

Because of the restrictions on DllMain (and I understood that the same applies to global&static object constructors&destructors in a DLL), such a simple thing as a singleton logger with asynchronous file writing/flushing thread becomes too tricky. The singleton logger is in a DLL, and I have limited influence on the executable loading and unloading this DLL time to time. I can force that executable to call my DLL initialization function before any use, so in the initialization function I can use a critical section to guard a variable telling whether the DLL has been already initialized or it needs init this time. This way initialization from DllMain is avoided, which would lead to a deadlock because I need to launch threads from initialization, and threads call DllMain with DLL_THREAD_ATTACH reason, and that obtains the same loader lock as the one already obtained when we are initializing in DllMain on DLL_PROCESS_ATTACH event.

C++11 thread cannot be used because of this bug (not fixed in MSVC++2013). So I'm using _beginthreadex(), because CreateThread documentation says:

A thread in an executable that calls the C run-time library (CRT) should use the _beginthreadex and _endthreadex functions for thread management rather than CreateThread and ExitThread; this requires the use of the multithreaded version of the CRT. If a thread created using CreateThread calls the CRT, the CRT may terminate the process in low-memory conditions.

But I have no control over the executable to ensure that some deinitialization function from the DLL is called before unloading the DLL. So the only options for cleanup are DllMain's DLL_PROCESS_DETACH and destructors of global/static variables. The problem is that they are called with loader lock obtained, so I cannot make there the DLL threads to exit gracefully because those threads upon a normal exit would attempt to call DllMain with DLL_THREAD_DETACH, which would result in a deadlock (loader lock again). MSDN suggests to use TerminateThread() to handle this:

DLL A gets a DLL_PROCESS_DETACH message in its DllMain and sets an event for thread T, signaling it to exit. Thread T finishes its current task, brings itself to a consistent state, signals DLL A, and waits infinitely. Note that the consistency-checking routines should follow the same restrictions as DllMain to avoid deadlocking. DLL A terminates T, knowing that it is in a consistent state.

So I'm afraid of using _beginthreadex() + TerminateThread() pair, instead of the designed _endthreadex() (the latter would be called by the thread itself if the thread returned normally).

tl;dr Consider a thread which returns from its entry function vs. a thread which does something like Sleep(INFINITE) in the end of its function waiting to get terminated (i.e. after it has got the resources consistent and signalled to the terminating thread that it's ready). Do some CRT or C++11 resources (like thread_local) etc. leak or get corrupted etc. if _endthreadex() is not called, but TerminatThread() is called instead?

like image 716
Serge Rogatch Avatar asked Aug 31 '16 05:08

Serge Rogatch


1 Answers

OK. First, let's cover a few minor points:

  • As David mentions in the comments, you don't need to use _beginthreadex() rather than CreateThread(). Similarly, it is fine to use ExitThread() or similar instead of _endthreadex() on any currently supported version of Visual Studio and Windows.

  • Despite what that MSDN article says, the accepted wisdom is that it is never OK to use TerminateThread().

  • It is also generally accepted that, provided you understand the restrictions implied by the loader lock, it is OK to use CreateThread() in DllMain's DLL_PROCESS_ATTACH processing. However, if you are able to use a proper initialization routine rather than DllMain, as in your case, so much the better.

So, if I've understood your situation correctly, it can be summarized as follows:

  • Your DLL needs one or more background threads.

  • The executable unloads your DLL without warning.

That's kind of stupid, but that's not your fault. Luckily it isn't impossible to deal with.

If it is acceptable for the thread(s) to continue running after the executable thinks it has unloaded your DLL, you can use the FreeLibraryAndExitThread() pattern. In your initialization function, and wherever else you create a thread, call GetModuleHandleEx() to increment the DLL reference count. That way, when the executable calls FreeLibrary() the DLL won't actually be unloaded if any of the threads are still running. The threads exit by calling FreeLibraryAndExitThread(), maintaining the reference count.

This approach might not directly meet your needs, however, because it doesn't allow you to detect when the executable has unloaded the library so that you can signal the thread(s) to terminate.

There may be more clever solutions, but I'd suggest using a helper DLL. The idea is that the helper DLL rather than your primary DLL keeps track of the thread reference count, i.e., you load the helper DLL each time you create a background thread, and unload it each time a background thread exits. The helper DLL need only contain a single function which calls SetEvent() and then FreeLibraryAndExitThread().

When a background thread is notified that the DLL is being unloaded, it cleans up, and then calls the helper DLL to set an event and exit the thread. Once the event is set, your main DLL's detach routine knows that the thread is no longer running code from the main DLL. Once every background thread has finished cleanup, it is safe for the main DLL to unload - it doesn't matter that the threads are still running, because they are running code from the helper DLL, not the main DLL. The helper DLL in turn will unload automatically once the last thread calls FreeLibraryAndExitThread().


Looking at this again, a year or so later, it would probably be safer to reverse that: have the main DLL contain nothing but the initialization function and whatever other functions the program is calling, plus a DllMain that signals the background threads to exit, and have a secondary DLL that contains everything else.

In particular, if the secondary DLL contains all the code your background threads need, then it is safe for the primary DLL to unload while the background threads are still running.

The advantage of this variant is that when your background threads see the signal to exit, it doesn't matter if the primary DLL has already unloaded, so your DllMain function doesn't have to wait while holding the loader lock. That way, the process won't deadlock if one of the background threads inadvertently does something that requires the loader lock.


As a variant on the same idea, if you really really don't want to use FreeLibraryAndExitThread() on your CRT threads, you could instead have an extra thread living in the secondary DLL that would coordinate the unloading. This thread would be started with CreateThread() and wouldn't use any CRT functions, so that it is unquestionably safe for it to exit via FreeLibraryAndExitThread(). Its only responsibility would be to wait for all the other threads to exit before unloading the secondary DLL.

It isn't really necessary to distinguish between CRT and non-CRT threads any more, but if you want to strictly follow the rules-as-documented this would be one way of doing it.

like image 167
Harry Johnston Avatar answered Nov 04 '22 11:11

Harry Johnston