[PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

[Ben Craig via cfe-commits]

bcraig added a comment.

In http://reviews.llvm.org/D21803#470060, @tavianator wrote:

> In http://reviews.llvm.org/D21803#469988, @bcraig wrote:
>
> > You should look at __thread_specific_ptr in libcxx's <thread>.  It does a lot of these things in order to satisfy the requirements of notify_all_at_thread_exit, set_value_at_thread_exit, and make_ready_at_thread_exit.
>
>
> Had a look at it.  One thing that stands out is that notify_all_at_thread_exit() and friends are supposed to be invoked *after* thread_local destructors.  But the order that pthread_key destructors run in is unspecified.  This could be worked around by waiting for the second iteration through pthread_key destructors before triggering ~__thread_struct_imp().  It looks like libstdc++ has a similar bug if ..._impl() isn't available.


It also intentionally leaks the pthread key.  Does the __thread_specific_ptr rationale hold for this change as well?

> > This implementation of __cxa_thread_atexit doesn't interact nicely with shared libraries.  The following sequence of events causes unloaded code to get invoked.

> 

> > 

> 

> > - Create thread 42

> 

> > - Load library foo from thread 42

> 

> > - Call a function with a thread_local object with a dtor.

> 

> > - Unload library foo.

> 

> > - Join thread 42

> 

> > 

> 

> >   glibc does some extra work during __cxa_thread_atexit_impl to bump the reference count of the shared library so that the user's "unload" doesn't actually unload the library.

> 

> 

> Yep.  This is about as good as libc++abi can do on its own though.  Note that libstdc++ has similar limitations if ..._impl() isn't available.


I was going to tell you that this is implementable with dladdr (which I think Android has).  Then I looked more at the "prevent unloading" side of things, and it looks like that requires digging into the library structures directly.  Ugh.

Comment on the limitation in the source, but you don't need to change any code for this item.


http://reviews.llvm.org/D21803