0

There is this multiplatform (Windows, Linux, Cygwin) dynamic library which is loaded at run time by a Cygwin executable. At some point of time, during the normal workflow, the DLL allocates a pool of threads for use. These threads are managed as global variables (reference counted). So when the client process goes to shutdown, it starts releasing global objects, threads should be released too.

Issue is, as I understand, that during the process shutdown, the Loader lock is acquired and further down the street, threads want to acuiqre the same lock and, we have now a deadlock.

Now my ask for advise is, how we can make a nice shutdown?

The DLL has no init() or uninit() methods to be called. The client at best can be enhanced with some code before the end of main () (so this is before the process shutdown).

If I detach the threads, instead of joining them, during the global var clean up, memory goes corrupted. If I terminate them, we have ugly process dumps.

Btw, under Linux I see no such problems.

DLL is only C++14, client is C99 (Cygwin).

I tried to make the situation clear, but let me know if you have further questions. Thanks in advance for any ideas.

Rado
  • 766
  • 7
  • 14
  • It looks like adding explicit `init()` and `uninit()` methods would be a best option here. – user7860670 Nov 06 '18 at 11:39
  • You may be right, but we really want to avoid explicit requirement on that. So there may be elegant enough solution to handle such situation. There should be. There should be! – Rado Nov 06 '18 at 12:01
  • DLL's can clean themselves up with [`FreeLibraryAndExitThread`](https://learn.microsoft.com/en-us/windows/desktop/api/libloaderapi/nf-libloaderapi-freelibraryandexitthread); this prevents a crash when `uninit` unloads its own DLL. – MSalters Nov 06 '18 at 12:10
  • https://stackoverflow.com/a/9759272/17034 – Hans Passant Nov 06 '18 at 12:31
  • @MSalters I think if we have `uninit()` called before `main()` ends, we can shutdown these threads without that function? Mind you, the DLL is `FreeLibrary()`-called much before the process shutdown (like still in client's `main()`). It is just that the global vars are scheduled for cleaning on CRT shutdown. – Rado Nov 06 '18 at 12:32
  • @HansPassant Thanks you! I have played with these, but to no success. I mean the various `exit()` functions. Point is that the thread is just simple loop with no storage on its own. So the difference between these functions does not help. – Rado Nov 06 '18 at 12:36
  • @Rado: If you call `FreeLibrary` on a library that still has global vars, the CRT will later schedule a call to the appropriate destructors which are already unloaded. That's a crash regardless of loader lock. – MSalters Nov 06 '18 at 12:36
  • @MSalters but then why the OS does not call the global vars destructors during `FreeLibrary`? I was relying on this mechanism at the beginning too. Instead the OS just calls the destructors after the exit of client's `main()`. The DLL is just at one place loaded and unloaded, so its ref count is ok. – Rado Nov 06 '18 at 12:41
  • @Rado: Destructors are a C++ concept; Windows is language-agnostic. So it doesn't call C++ destructors for the same reason it doesn't try to do Java garbage collection. – MSalters Nov 06 '18 at 12:44
  • @MSalters: Sorry, I think I didn't get what you tried to say with the `FreeLibrary` up there. My point was that the `FreeLibrary` does nothing to clean up the global vars, instead this is postponed for the process end. There is no crash, just deadlock. – Rado Nov 06 '18 at 12:50
  • @Rado: The point is that `FreeLibrary` doesn't call the destructors, so the C++ runtime tries to arrange for that. But the C++ runtime does assume that it can still call those destructors, and that those destructors are safe under loader lock. Failing either assumption causes problems. And if the code is wrong in both aspects, it's 50/50 which problem occurs first. – MSalters Nov 06 '18 at 12:55
  • @MSalters: Got it.So basically add `uninit()` before `FreeLibrary()`. – Rado Nov 06 '18 at 13:03

2 Answers2

0

The fix is to add an uninit method to the DLL. It may not have one yet, but it needs one. You found out why: while the OS will call DllMain on DLL unload, it does so under loader lock. You need to do things that aren't possible under loader lock, so you need an extra call before DllMain. Naming that method uninit() is reasonable enough.

C++14 is not an issue here; this is an OS mechanism. Loader Lock has been around since ancient times.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • Isn't there an way to hook between the end of `main()` and before that lock aquisition? It is so okay behaving this flow on Linux... – Rado Nov 06 '18 at 12:37
  • @Rado: Well, obviously if you declare an object inside `main`, then its destructor will run at the end of `main`. – MSalters Nov 06 '18 at 12:40
  • I can't touch that piece of code. I see your point - require `uninit()` during `main()`, and I thought of that, but I search for other possible solutions too. Thank you. – Rado Nov 06 '18 at 12:48
0

At a previous job I struggled with this issue for a pretty lengthy period. Ultimately it came down to only 2 possible solutions:

  1. Clean up every single resource the thread has claim to, then TerminateThread. It's violent and ugly but it works around the THREAD_DETACH issue and I actually found it advised on the Internet.
  2. If you have the luxury of being able to get advance notice prior to PROCESS_DETACH - clean up everything at that early point, including orderly shutdown of your threads. Then by all means do absolutely nothing during PROCESS_DETACH - yes, don't even free any lingering heap objects, as you may be exposing yourself to deadlock or crash risks and the process is going down and freeing up all its resources anyway.

As an added note, I also learned to avoid at all costs having any global variables linked to the DLL lifetime. These will have their constructors and destructors executed in the DllMain context, needless to say more... If you need global singletons in a DLL, make sure to have manual control on their lifetimes (on both ends, so no auto-destruct smart pointers either).

cpl
  • 26
  • 3
  • Thanks for sharing! I think point 2) is usually used - like using `uninit()`. And for your last remark - `init()` is used. These threads are quite lightweight. I tried with termination also, but always got some ugly system messages or dumps with the different methods and somehow - I felt bad with it. I wonder why on Linux for example there are no such problems?! Anyway, current requirement is to use `uninit()` call, just in front of `FreeLibrary()`. – Rado Nov 17 '18 at 05:15
  • One crucial point I forgot to make about #2: My advice not to clean up is only valid if the process is actually going down vs. only the dll being unloaded, which may be determined from the value of lpReserved (MS must be real proud of this one...) – cpl Nov 30 '18 at 12:25