[libcxx] r245334 - [libc++] Fix PR22606 - Leak pthread_key with static storage duration to ensure all of thread-local destructors are called.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 18 12:40:38 PDT 2015


Author: ericwf
Date: Tue Aug 18 14:40:38 2015
New Revision: 245334

URL: http://llvm.org/viewvc/llvm-project?rev=245334&view=rev
Log:
[libc++] Fix PR22606 - Leak pthread_key with static storage duration to ensure all of thread-local destructors are called.

Summary:
See https://llvm.org/bugs/show_bug.cgi?id=22606 for more discussion.

Most of the changes in this patch are file reorganization to help ensure assumptions about how __thread_specific_pointer is used hold. The assumptions are:

* `__thread_specific_ptr<Tp>` is only created with a `__thread_struct` pointer.
* `__thread_specific_ptr<Tp>` can only be constructed inside the `__thread_local_data()` function.

I'll remove the comments before committing. They are there for clarity during review.

Reviewers: earthdok, mclow.lists

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D8802

Modified:
    libcxx/trunk/include/thread

Modified: libcxx/trunk/include/thread
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/thread?rev=245334&r1=245333&r2=245334&view=diff
==============================================================================
--- libcxx/trunk/include/thread (original)
+++ libcxx/trunk/include/thread Tue Aug 18 14:40:38 2015
@@ -113,11 +113,38 @@ void sleep_for(const chrono::duration<Re
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+template <class _Tp> class __thread_specific_ptr;
+class _LIBCPP_TYPE_VIS __thread_struct;
+class _LIBCPP_HIDDEN __thread_struct_imp;
+class __assoc_sub_state;
+
+_LIBCPP_FUNC_VIS __thread_specific_ptr<__thread_struct>& __thread_local_data();
+
+class _LIBCPP_TYPE_VIS __thread_struct
+{
+    __thread_struct_imp* __p_;
+
+    __thread_struct(const __thread_struct&);
+    __thread_struct& operator=(const __thread_struct&);
+public:
+    __thread_struct();
+    ~__thread_struct();
+
+    void notify_all_at_thread_exit(condition_variable*, mutex*);
+    void __make_ready_at_thread_exit(__assoc_sub_state*);
+};
+
 template <class _Tp>
 class __thread_specific_ptr
 {
     pthread_key_t __key_;
 
+     // Only __thread_local_data() may construct a __thread_specific_ptr
+     // and only with _Tp == __thread_struct.
+    static_assert(is_same<_Tp, __thread_struct>::value, "");
+    __thread_specific_ptr();
+    friend _LIBCPP_FUNC_VIS __thread_specific_ptr<__thread_struct>& __thread_local_data();
+
     __thread_specific_ptr(const __thread_specific_ptr&);
     __thread_specific_ptr& operator=(const __thread_specific_ptr&);
 
@@ -125,7 +152,6 @@ class __thread_specific_ptr
 public:
     typedef _Tp* pointer;
 
-    __thread_specific_ptr();
     ~__thread_specific_ptr();
 
     _LIBCPP_INLINE_VISIBILITY
@@ -159,7 +185,10 @@ __thread_specific_ptr<_Tp>::__thread_spe
 template <class _Tp>
 __thread_specific_ptr<_Tp>::~__thread_specific_ptr()
 {
-    pthread_key_delete(__key_);
+    // __thread_specific_ptr is only created with a static storage duration
+    // so this destructor is only invoked during program termination. Invoking
+    // pthread_key_delete(__key_) may prevent other threads from deleting their
+    // thread local data. For this reason we leak the key.
 }
 
 template <class _Tp>
@@ -307,26 +336,6 @@ public:
     static unsigned hardware_concurrency() _NOEXCEPT;
 };
 
-class __assoc_sub_state;
-
-class _LIBCPP_HIDDEN __thread_struct_imp;
-
-class _LIBCPP_TYPE_VIS __thread_struct
-{
-    __thread_struct_imp* __p_;
-
-    __thread_struct(const __thread_struct&);
-    __thread_struct& operator=(const __thread_struct&);
-public:
-    __thread_struct();
-    ~__thread_struct();
-
-    void notify_all_at_thread_exit(condition_variable*, mutex*);
-    void __make_ready_at_thread_exit(__assoc_sub_state*);
-};
-
-_LIBCPP_FUNC_VIS __thread_specific_ptr<__thread_struct>& __thread_local_data();
-
 #ifndef _LIBCPP_HAS_NO_VARIADICS
 
 template <class _Fp, class ..._Args, size_t ..._Indices>




More information about the cfe-commits mailing list