thread specific key leakage

[llhe]

Description

We build a program which use ostream and when we do stress testing by repeated loading and unloading the built shared library, the program crashed with an abort message cannot create thread specific key for __cxa_get_globals() here:

namespace __cxxabiv1 {
namespace {
    std::__libcpp_tls_key key_;
    std::__libcpp_exec_once_flag flag_ = _LIBCPP_EXEC_ONCE_INITIALIZER;

    void destruct_ (void *p) {
        __free_with_fallback ( p );
        if ( 0 != std::__libcpp_tls_set ( key_, NULL ) )
            abort_message("cannot zero out thread value for __cxa_get_globals()");
        }

    void construct_ () {
        if ( 0 != std::__libcpp_tls_create ( &key_, destruct_ ) )
            abort_message("cannot create thread specific key for __cxa_get_globals()");
        }
} 

which means pthread_key_create returns error.

// ./cxx-stl/llvm-libc++/include/__threading_support

377 // Thread local storage                                                         
378 int __libcpp_tls_create(__libcpp_tls_key *__key, void (*__at_exit)(void *))     
379 {                                                                               
380   return pthread_key_create(__key, __at_exit);                                  
381 }  

And checked the source of bionic implementation for pthread_key_create https://github.com/aosp-mirror/platform_bionic/blob/master/libc/bionic/pthread_key.cpp#L120:

__BIONIC_WEAK_FOR_NATIVE_BRIDGE
int pthread_key_create(pthread_key_t* key, void (*key_destructor)(void*)) {
  for (size_t i = 0; i < BIONIC_PTHREAD_KEY_COUNT; ++i) {
    uintptr_t seq = atomic_load_explicit(&key_map[i].seq, memory_order_relaxed);
    while (!SeqOfKeyInUse(seq)) {
      if (atomic_compare_exchange_weak(&key_map[i].seq, &seq, seq + SEQ_INCREMENT_STEP)) {
        atomic_store(&key_map[i].key_destructor, reinterpret_cast<uintptr_t>(key_destructor));
        *key = i | KEY_VALID_FLAG;
        return 0;
      }
    }
  }
  return EAGAIN;
}

Which means it will fail when there is no free slot (with 130 as the max).

And from the code above, there is no matching pthread_key_delete in destruct_ call. This will create a leak for each loading.

Environment Details

The program is based on 17b which is not the latest, however, the source code section should be enough.

• NDK Version: 17b
• Build system: bazel and cmake
• Host OS: ubuntu
• Compiler: clang
• ABI: armeabi-v7a
• STL: libc++
• NDK API level: 21
• Device API level: 27

[DanAlbert]

And from the code above, there is no matching pthread_key_delete in destruct_ call.

It's a little bit trickier than this because destruct_ will be called on the exit of any thread that used the key. There may be other threads still using the key, so we can't delete it. Ref counting the key in libc++abi gets a little bit messy, but may be an option.

Why are you looping dlopen/dlclose? I'm wondering if your stress tests are going to all be useless on modern devices where dlclose won't do anything for libraries with non-trivial thread-local destructors.

[llhe]

It makes sense.

We are library provider, it's our user who make such stress testing. So we can suggest them to skip this test case. However, the stress testing do make sense when the final product is a plugin/shared library.

[tavianator]

libc++ also intentionally leaks the pthread_keys used for notify_all_at_thread_exit() and __cxa_thread_atexit(). dlclose() on libc++(abi) is not really supported. Maybe you could load it with RTLD_NODELETE?

[DanAlbert]

Forgot to close this one. Like @tavianator said, these are intentionally leaked by libc++abi as a necessity of the fallback __cxa_thread_atexit_imp. See https://reviews.llvm.org/D21803 for more discussion.

Devices running 23+ will get the non-fallback behavior. If you need to avoid the leak for earlier platforms, there are a few workarounds:

1. Don't use dlclose
2. Use RTLD_NODELETE
3. Don't use thread_local for globals with non-trivial destructors

[olegarch]

Thanks for detailed explanation. I have a follow up question.

1. Don't use dlclose
2. Use RTLD_NODELETE
3. Don't use thread_local for globals with non-trivial destructors

If 1. and 2. are not an option, does 3. mean not using std::thread at all?

[DanAlbert]

does 3. mean not using std::thread at all?

No, you can use std::thread all you want. You just can't use thread_local on globals with non-trivial destructors if you care about old OSs.

But if 1 isn't an option you should probably re-evaluate your implementation. dlclose cannot be relied upon to do anything at all. thread_local globals with non-trivial destructors are one reason that it might do nothing for the current implementation of Android's loader, but that could change in the future to enable new features, or could just become a no-op at some point because that's the only valid implementation with predictable behavior.