Fix folly/ThreadLocal with clang after PthreadKeyUnregister change

Summary: Make PthreadKeyUnregister a global singleton for all StaticMeta instances.

Reviewed By: luciang

Differential Revision: D2678401

fb-gh-sync-id: c0d58b483fa6f096b29aeb7df71897a75ea8c892

diff --git a/folly/Makefile.am b/folly/Makefile.am
index 6639093214f4e1ce82166405d9dcf9cb6273aeea..ed749242fa4bcf78b1f90b6b8e4e5633055a32af 100644 (file)
--- a/folly/Makefile.am
+++ b/folly/Makefile.am
@@ -330,6 +330,7 @@ libfolly_la_SOURCES = \
        futures/QueuedImmediateExecutor.cpp \
        futures/ThreadWheelTimekeeper.cpp \
        detail/Futex.cpp \
+       detail/ThreadLocalDetail.cpp \
        GroupVarint.cpp \
        GroupVarintTables.cpp \
        IPAddress.cpp \

diff --git a/folly/Portability.h b/folly/Portability.h
index bdac6cc33fe714e24a75baed89d2585940923bad..d0c37d9b5aac56dc014a80d6c6f8a1eda3da3909 100644 (file)
--- a/folly/Portability.h
+++ b/folly/Portability.h
@@ -397,5 +397,14 @@ constexpr size_t constexpr_strlen(const char* s) {
 #endif
 }
 
+#if defined(__APPLE__) || defined(_MSC_VER)
+#define MAX_STATIC_CONSTRUCTOR_PRIORITY
+#else
+// 101 is the highest priority allowed by the init_priority attribute.
+// This priority is already used by JEMalloc and other memory allocators so
+// we will take the next one.
+#define MAX_STATIC_CONSTRUCTOR_PRIORITY __attribute__ ((__init_priority__(102)))
+#endif
+
 } // namespace folly
 #endif // FOLLY_PORTABILITY_H_

diff --git a/folly/detail/ThreadLocalDetail.cpp b/folly/detail/ThreadLocalDetail.cpp
new file mode 100644 (file)
index 0000000..5815636
--- /dev/null
+++ b/folly/detail/ThreadLocalDetail.cpp
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2015 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <folly/ThreadLocal.h>
+
+namespace folly { namespace threadlocal_detail {
+
+PthreadKeyUnregister
+MAX_STATIC_CONSTRUCTOR_PRIORITY PthreadKeyUnregister::instance_;
+
+}}

diff --git a/folly/detail/ThreadLocalDetail.h b/folly/detail/ThreadLocalDetail.h
index da3ee0732c1647a9f1b8f1769acf9f29b2e42929..c97083ffecff20aac572213a7863d0e4ef78cbf0 100644 (file)
--- a/folly/detail/ThreadLocalDetail.h
+++ b/folly/detail/ThreadLocalDetail.h
@@ -163,6 +163,43 @@ struct ThreadEntry {
 
 constexpr uint32_t kEntryIDInvalid = std::numeric_limits<uint32_t>::max();
 
+/**
+ * We want to disable onThreadExit call at the end of shutdown, we don't care
+ * about leaking memory at that point.
+ *
+ * Otherwise if ThreadLocal is used in a shared library, onThreadExit may be
+ * called after dlclose().
+ */
+class PthreadKeyUnregister {
+ public:
+  ~PthreadKeyUnregister() {
+    std::lock_guard<std::mutex> lg(mutex_);
+
+    for (const auto& key: keys_) {
+      pthread_key_delete(key);
+    }
+  }
+
+  static void registerKey(pthread_key_t key) {
+    instance_.registerKeyImpl(key);
+  }
+
+ private:
+  PthreadKeyUnregister() {}
+
+  void registerKeyImpl(pthread_key_t key) {
+    std::lock_guard<std::mutex> lg(mutex_);
+
+    keys_.push_back(key);
+  }
+
+  std::mutex mutex_;
+  std::vector<pthread_key_t> keys_;
+
+  static PthreadKeyUnregister instance_;
+};
+
+
 // Held in a singleton to track our global instances.
 // We have one of these per "Tag", by default one for the whole system
 // (Tag=void).
@@ -251,6 +288,7 @@ struct StaticMeta {
     head_.next = head_.prev = &head_;
     int ret = pthread_key_create(&pthreadKey_, &onThreadExit);
     checkPosixError(ret, "pthread_key_create failed");
+    PthreadKeyUnregister::registerKey(pthreadKey_);
 
 #if FOLLY_HAVE_PTHREAD_ATFORK
     ret = pthread_atfork(/*prepare*/ &StaticMeta::preFork,

diff --git a/folly/test/ThreadLocalTest.cpp b/folly/test/ThreadLocalTest.cpp
index 82e6eb4f8cf3097d379c32ac25d11b621fe04bd6..823c6cb515a778443030f4f8ff6c51bcc726075f 100644 (file)
--- a/folly/test/ThreadLocalTest.cpp
+++ b/folly/test/ThreadLocalTest.cpp
@@ -16,6 +16,7 @@
 
 #include <folly/ThreadLocal.h>
 
+#include <dlfcn.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -37,6 +38,7 @@
 #include <gtest/gtest.h>
 
 #include <folly/Benchmark.h>
+#include <folly/Baton.h>
 
 using namespace folly;
 
@@ -539,6 +541,47 @@ TEST(ThreadLocal, Fork2) {
   }
 }
 
+TEST(ThreadLocal, SharedLibrary)
+{
+  auto handle = dlopen("./_bin/folly/test/lib_thread_local_test.so",
+                       RTLD_LAZY);
+  EXPECT_NE(nullptr, handle);
+
+  typedef void (*useA_t)();
+  dlerror();
+  useA_t useA = (useA_t) dlsym(handle, "useA");
+
+  const char *dlsym_error = dlerror();
+  EXPECT_EQ(nullptr, dlsym_error);
+
+  useA();
+
+  folly::Baton<> b11, b12, b21, b22;
+
+  std::thread t1([&]() {
+      useA();
+      b11.post();
+      b12.wait();
+    });
+
+  std::thread t2([&]() {
+      useA();
+      b21.post();
+      b22.wait();
+    });
+
+  b11.wait();
+  b21.wait();
+
+  dlclose(handle);
+
+  b12.post();
+  b22.post();
+
+  t1.join();
+  t2.join();
+}
+
 // clang is unable to compile this code unless in c++14 mode.
 #if __cplusplus >= 201402L
 namespace {

diff --git a/folly/test/ThreadLocalTestLib.cpp b/folly/test/ThreadLocalTestLib.cpp
new file mode 100644 (file)
index 0000000..1a81528
--- /dev/null
+++ b/folly/test/ThreadLocalTestLib.cpp
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2015 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <iostream>
+#include <thread>
+
+#include <folly/ThreadLocal.h>
+
+class A {
+ public:
+  void use() const {
+  }
+};
+
+folly::ThreadLocal<A> a;
+
+extern "C" {
+
+void useA() {
+  a->use();
+}
+
+}