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

Eric Fiselier eric at efcs.ca
Thu Apr 2 09:18:10 PDT 2015


Hi mclow.lists, earthdok,

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.

http://reviews.llvm.org/D8802

Files:
  include/thread

Index: include/thread
===================================================================
--- include/thread
+++ include/thread
@@ -113,11 +113,38 @@
 
 _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 @@
 public:
     typedef _Tp* pointer;
 
-    __thread_specific_ptr();
     ~__thread_specific_ptr();
 
     _LIBCPP_INLINE_VISIBILITY
@@ -159,6 +185,10 @@
 template <class _Tp>
 __thread_specific_ptr<_Tp>::~__thread_specific_ptr()
 {
+    // __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.
     pthread_key_delete(__key_);
 }
 
@@ -307,26 +337,6 @@
     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>

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D8802.23157.patch
Type: text/x-patch
Size: 2610 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150402/2a5eefb0/attachment.bin>


More information about the cfe-commits mailing list