Boost C++ Libraries: Ticket #3926: thread_specific_ptr + dlopen library causes a SIGSEGV.

attachment set

anonymous — Fri, 12 Feb 2010 23:05:56 GMT

attachment → tls-test.boost.tgz

status changed; resolution set

Anthony Williams — Mon, 15 Mar 2010 16:03:55 GMT

status new → closed
resolution → wontfix

When you dlclose a library, you must ensure that no code references that library afterwards. If you have a live thread_specific_ptr with a destructor that belongs to that library then the library must be loaded when the destructor is called. You must therefore ensure that the thread_specific_ptr does not have a value on any thread when the library is unloaded.

status changed; resolution deleted

pluto@… — Mon, 07 Jun 2010 19:29:53 GMT

status closed → reopened
resolution wontfix

Replying to anthonyw:

When you dlclose a library, you must ensure that no code references that library afterwards. If you have a live thread_specific_ptr with a destructor that belongs to that library then the library must be loaded when the destructor is called. You must therefore ensure that the thread_specific_ptr does not have a value on any thread when the library is unloaded.

the thread_specific_ptr dtor called from dlclose() calls set_tss_data() with boost default cleanup function and *NULL* new_value. what's the point of deleting NULL pointer in cleanup function? this accidentally works in shared libs enviroment beacause the cleanup function exists in memory and 'delete 0' is valid in c++. with dynamically loaded libs glibc tries to call useless cleanup callback from dl-unloaded code and ends with gpf. solution: the thread_specific_ptr dtor should delete current_value and call set_tss_data() in the same way as the release() does.

pluto@… — Thu, 10 Jun 2010 14:45:58 GMT

currently there's a crash in boost.thread core. please follow this scenario:

start thread.
  dlopen lib.
    run function from lib which uses thread_specific_ptr.
      boost::detail::create_epoch_tss_key registers delete_epoch_tss_data destructor.
    exit from lib function.
  dlclose lib (unload delete_epoch_tss_data code).
exit thread
  __nptl_deallocate_tsd tries to call unloaded key destructor and crashes.

i think that libs/thread/src/pthread/once.cpp should contain alternative cleanup function with attributte(( destructor )):

if ( pthread_getspecific( epoch_tss_key ) )
  pthread_key_delete( epoch_tss_key )

attachment set

pluto@… — Fri, 11 Jun 2010 13:14:57 GMT

attachment → boost-tls.patch

proposed patch for proper cleanup of the static libboost_thread linked into shared objects.

dadrus — Thu, 28 Apr 2011 10:45:05 GMT

The patch provided above is not applicable on apple. At least on Mac OSX 10.5 PTHREAD_ONCE_INIT is defined as

#define PTHREAD_ONCE_INIT {_PTHREAD_COND_SIG_init, {0}}

and not as

#define PTHREAD_ONCE_INIT 0

like on Linux, so one can not use simple compare operator. Here comes the patch, which works on both, linux and mac. I'm still using 1.38 version of boost. So one has to adjust it for newer versions.

attachment set

dadrus — Thu, 28 Apr 2011 10:45:53 GMT

attachment → boost_thread.patch

patch for linux and mac

dadrus — Thu, 28 Apr 2011 10:57:35 GMT

There is a small typo in my last comment. On Mac PTHREAD_ONCE_INIT is defined as follows:

#define PTHREAD_ONCE_INIT {_PTHREAD_ONCE_SIG_init, {0}}

I would like also to ask you Anthony to tackle this ticket soon. Hopefully for the next boost release.

Regards

Dimitrij

tkramer@… — Fri, 22 Jul 2011 12:28:28 GMT

I ran into this recently as well. Any updates on getting this patch committed?

owner, status, type, milestone changed

viboes — Wed, 07 Dec 2011 18:24:27 GMT

owner changed from Anthony Williams to viboes
status reopened → new
type Bugs → Patches
milestone Boost 1.43.0 → To Be Determined

viboes — Wed, 07 Dec 2011 22:20:59 GMT

I don't know nothing about dl_open/dl_close. Please, could you explain me when and why delete_epoch_tss_key_on_dlclose will be called?

viboes — Wed, 07 Dec 2011 22:29:38 GMT

Oh, I see now it is the gcc attributte(( destructor )) that make it be called after main().

Shouldn't this function be added conditionally (when this attribute is available)?

What about compilers that don't support this attribute? How the the key will be deleted?

status changed

viboes — Wed, 07 Dec 2011 22:31:57 GMT

status new → assigned

viboes — Sun, 11 Dec 2011 02:07:27 GMT

See also #4639 boost thread library leaks pthread_key

pluto@… — Sun, 11 Dec 2011 09:28:52 GMT

Replying to viboes:

See also #4639 boost thread library leaks pthread_key

yes, the #4639 describes the same problem but there's one major issue with both patches. the desctruction order of global objects is not specified in general, especially in these days when recent gnu toolchain can sort .init/.fini-array sections.

e.g. the ~delete_epoch_tss_key_on_dlclose_t() from #4639 may release pthread_key and after this the ~thread_specific_ptr() may accuire key again, set null handler and finally leak/gpf as usual.

viboes — Sat, 07 Jan 2012 23:20:57 GMT

See also #2470 Problem with threads started in a dynamically loaded bundle under Mac OS X

viboes — Fri, 06 Sep 2013 05:50:46 GMT

Replying to pluto@…:

Replying to viboes:

See also #4639 boost thread library leaks pthread_key

yes, the #4639 describes the same problem but there's one major issue with both patches. the desctruction order of global objects is not specified in general, especially in these days when recent gnu toolchain can sort .init/.fini-array sections.

e.g. the ~delete_epoch_tss_key_on_dlclose_t() from #4639 may release pthread_key and after this the ~thread_specific_ptr() may accuire key again, set null handler and finally leak/gpf as usual.

kai.dietrich@… — Mon, 03 Feb 2014 16:28:40 GMT

Hi all,

I also got bitten by that one. Our application compiles boost statically into a shared library (.so/linux). Users are supposed to dl_open/dl_close (potentially often). Each open/close cycle leads to a key/handle leak due to boost not freeing the handle and not giving any control over the tls handles to the user.

At the moment the tls keys in thread.cpp and once.cpp are internal and not accessible from the outside. Would it be possible to give out a way to the user. The windows implementation also has some pretty verbose tls cleanup code.

This bug can be a real production showstopper. I'd give it a way higher priority.

viboes — Tue, 04 Feb 2014 02:28:10 GMT

Replying to kai.dietrich@…:

Hi all,

I also got bitten by that one. Our application compiles boost statically into a shared library (.so/linux). Users are supposed to dl_open/dl_close (potentially often). Each open/close cycle leads to a key/handle leak due to boost not freeing the handle and not giving any control over the tls handles to the user.

At the moment the tls keys in thread.cpp and once.cpp are internal and not accessible from the outside. Would it be possible to give out a way to the user. The windows implementation also has some pretty verbose tls cleanup code.

This bug can be a real production showstopper. I'd give it a way higher priority.

I know and any help in this direction is welcome.

anonymous — Tue, 04 Feb 2014 07:45:47 GMT

As said, the win32 implementation has this:

  namespace boost {
    namespace detail {
      BOOST_THREAD_DECL void __cdecl on_process_enter()
      {}
      BOOST_THREAD_DECL void __cdecl on_thread_enter()
      {}
      BOOST_THREAD_DECL void __cdecl on_process_exit()
      {
        boost::cleanup_tls_key();
      }
      BOOST_THREAD_DECL void __cdecl on_thread_exit()
      {
        ...
      }
    }
  }

These functions are then somehow added into the PE image destructor sections (tss_dll.cpp and tss_pe.cpp). This was also buggy in some boost versions and messed up MFC cleanup (which also a tls handle leak) so we just disabled that and call the on_process_exit() function manually.

viboes — Tue, 08 Sep 2015 17:11:21 GMT

It seems that #11302 contains a patch that solves the issue.

Please, could you check it?

viboes — Tue, 08 Sep 2015 19:24:06 GMT

https://github.com/boostorg/thread/commit/242cf35c519fc886c13789f1f9a049571fde4cdc

pawels@… — Tue, 08 Sep 2015 19:30:58 GMT

Replying to viboes:

It seems that #11302 contains a patch that solves the issue.

Please, could you check it?

i've reported this testcase many years ago :-) it works with recent patch. please commit and close both tickets.

viboes — Wed, 23 Sep 2015 23:28:48 GMT

https://github.com/boostorg/thread/commit/242cf35c519fc886c13789f1f9a049571fde4cdc

viboes — Wed, 23 Sep 2015 23:29:39 GMT

So, I can define by default BOOST_THREAD_PATCH.

milestone changed

viboes — Wed, 23 Sep 2015 23:34:03 GMT

milestone To Be Determined → Boost 1.60.0

type changed

viboes — Wed, 23 Sep 2015 23:35:48 GMT

type Patches → Bugs

status changed; resolution set

viboes — Sun, 27 Sep 2015 13:35:58 GMT

status assigned → closed
resolution → fixed

https://github.com/boostorg/thread/commit/730cb550e6d969520c3f39e0ddf5d80351bddf3a

status changed; resolution, milestone deleted

viboes — Thu, 31 Mar 2016 22:13:53 GMT

status closed → reopened
resolution fixed
milestone Boost 1.60.0

This last change has been rolled back as it is introduce a regression in #12049

https://github.com/boostorg/thread/commit/47357de276fe4fc01469f34c1dbf8b26fdbc1c4b

Please, add BOOST_THREAD_PATCH if you need it absolutely.