<div dir="ltr">So unfortunately this looks tricker than I had hoped to change the shutdown order.  My plan was to add a function called llvm_register_shutdown_handler() that was basically a replacement for atexit(), which would allow us to run the shutdown handlers in llvm_shutdown().  Unfortunately there are numerous calls scattered throughout the codebase which call exit(), which kills this approach unless we create a new function called llvm_exit() which first calls llvm_shutdown() and then calls exit, and then replace all calls to exit() with llvm_exit().  But then that leaves open the possibility that someone else doesn't know about llvm_exit(), and adds more calls to exit() in the future.<div>
<br></div><div>I'm not really sure what a good solution is.  Assuming the patch that's currently uploaded passes the tests on linux and Mac, I'm inclined to special-case this and just use a critical section for this one specific mutex on Windows-only, and leave everything else using the STL implementation, adding a comment to the class to indicate that it cannot be used on Windows during an atexit handler.</div>
<div><br></div><div>At a higher level, I think ti would be great if the system were architected such that atexit() handlers were never used.  They're error-prone (as we see here), and seem to me like a bandaid for poorly designed shutdown / cleanup strategy.  Having only submitted a handful of trivial patches before and only having looked at llvm code for the first time a few days ago though, I'm probably not the right person to do that.  Maybe in the future :P</div>
<div><br></div><div><br></div><div>One more question: When I run the tests from inside Visual Studio, I get a bunch of errors about not being able to find a bunch of linux utilities like grep / bash / etc.  Is this normal?  Is it possible to run the test suite without cygwin?</div>
<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, May 30, 2014 at 8:31 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thank you for digging so deeply into this!<br>
<div class=""><br>
On Thu, May 29, 2014 at 8:23 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> So, the issue is that std::mutex and std::recursive_mutex simply cannot be<br>
> used during an atexit() handler.  It still seems like a good drop-in<br>
> replacement for the hand-rolled mutex in all other cases.<br>
><br>
> This is not documented anywhere, but if you dig into the code you'll find<br>
> that std::mutex and std::recursive_mutex are implemented using ConcRT<br>
> (microsoft concurrency runtime), and it relies on a call to<br>
> RegisterWaitForSingleObject().  That function in turn uses QueueUserAPC, and<br>
> if you check the documentation you'll see that it says "When the thread is<br>
> in the process of being terminated, calling QueueUserAPC to the thread's APC<br>
> queue will fail with ERROR_GEN_FAILURE (31)".<br>
><br>
> So, TL;DR - can't use mutexes from atexit handler.  This seems like a<br>
> reasonable restriction, as atexit handlers are probably pretty rare anyway.<br>
<br>
</div>I agree that this is a reasonable restriction.<br>
<div class=""><br>
> I will try to find a way to change the shutdown order to not rely on the<br>
> atexit handler.<br>
<br>
</div>Sounds like a good approach.<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div>