[PATCH] PrettyStackTrace should use ManagedStatic for its ThreadLocal

[Reid Kleckner]

On Thu, Sep 12, 2013 at 2:13 PM, Filip Pizlo <fpizlo at apple.com> wrote:

>
> How about adding a variant of ManagedStatic that registers the destructor
> with atexit(), just like C++ dynamic initializers do, instead of the LLVM
> shutdown machinery?
>
>
> That wouldn't fix the problem that I'm trying to fix.  A library that
> wishes to be used in a larger system shouldn't register atexit() or
> anything like it.  That's why I like that ManagedStatic's are only
> destroyed via an explicit call to llvm_shutdown().
>
> Registering atexit() means that you end up with the following situation:
>
> Program A links against library B, which then links against library C.
>  Lets say C = llvm.
>
> Library B provides a create/destroy API for its own objects.  It does not
> require programs that use it to call destroy, if the object is used as a
> global.  This is common.  The code base I work with (JavaScriptCore, part
> of WebKit, see webkit.org) has provided just such an API for a long time
> and we intend to continue supporting it.
>
> Library B decides to spawn LLVM compilations concurrently.
>
> Now you get the following exit-time race:
>
> 1) B tells LLVM to compile something on a thread.
> 2) LLVM starts working.
> 3) A calls exit(0).
>
> exit() then calls atexit() handlers and you start destroying
> ManagedStatic's while LLVM is working in a thread.
>
> One of the things that my patches are trying to ensure is that if the
> client of LLVM (in this case library B) isn't itself shut down explicitly
> (something that is common and that we allow with our APIs) then you don't
> get exit-time races, because then the client won't call llvm_shutdown().
>  In fact, we're going further in JavaScriptCore: we never call
> llvm_shutdown(), though we do destroy the Context/ExecutionEngine/etc if
> our notion of context (we call it "VM") is destroyed or if we haven't used
> LLVM for a while.  Of course, we won't destroy the
> Context/ExecutionEngine/etc if our client calls exit() while we're using
> them on a thread - and we'd like this to Just Work.
>
> I know that you could argue that clients of LLVM should always call
> llvm_shutdown().  This will probably work for a lot of clients where the
> relationship between the outer application and LLVM isn't transitive
> through multiple intervening libraries.  We certainly can't change our API
> to require our client to destroy the global VM, and it would be silly if
> that blocked us from ever using LLVM to compile any code due to exit-time
> crashes.
>

You know, clang might not even call llvm_shutdown() if -disable_free is
set, and that should be fine.  :)

>
> Then we could convert llvm::outs() to use it, thereby potential
> initialization races on Windows.
>
> Er, "thereby *fixing* potential races"

Interesting.  Can you elaborate what the problem is there?
>

llvm::outs() has a static local that closes stdout in its destructor.
 Obviously, this *really* has to run atexit, and probably can't run at
llvm_shutdown() time.  Alternatively, it shouldn't close stdout.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130912/5ca521f5/attachment.html>