[PATCH] D70447: [Support] ThreadPoolExecutor fixes for Windows/MinGW

Wed Dec 4 05:48:16 PST 2019

andrewng added a comment.

> - Stop the threads on llvm_shutdown if and when that is called, but do not deallocate any memory.

To be clear (and as stated in the code comment), this only stops the threads from taking on more tasks and does not actually wait for running threads to finish. The important thing that does occur, is that it ensures there is no further thread creation going on which could cause a crash on Windows as the main thread is doing process exit.

> - Deallocate the memory if and when the image is unloaded via a static unique_ptr.

This also joins the threads, again to avoid intermittent crashes on Windows.

> I see that LLD today calls llvm_shutdown to destroy ManagedStatics:
>  https://github.com/llvm/llvm-project/blob/master/lld/Common/ErrorHandler.cpp#L59
>  Please update that comment to indicate that this will also stop any threads that have been spawned, and this is critical for avoiding crashes on shutdown. It's actually a waste of time to deallocate other ManagedStatics, so someone might try to remove this at some point.

I will update the comment. This is already needed for `-time-passes` for LTO and I'm not sure it's entirely safe to generalize that all ManagedStatics do not need to be "shutdown".

> However, I wonder if LLD (and any other hypothetical clients of Parallel.h) should explicitly cancel, join, or detach threads during shutdown instead.

The only truly "safe" option is to always join threads on exit. This should avoid any possible undefined behaviour. However, this has been ruled out for performance reasons :(.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70447/new/

https://reviews.llvm.org/D70447