From 6982589748b6816ef68e6b53cb247dd099da08fe Mon Sep 17 00:00:00 2001 From: Andrii Grynenko Date: Fri, 20 Nov 2015 21:25:11 -0800 Subject: [PATCH] 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 --- folly/Makefile.am | 1 + folly/Portability.h | 9 +++++++ folly/detail/ThreadLocalDetail.cpp | 23 ++++++++++++++++ folly/detail/ThreadLocalDetail.h | 38 ++++++++++++++++++++++++++ folly/test/ThreadLocalTest.cpp | 43 ++++++++++++++++++++++++++++++ folly/test/ThreadLocalTestLib.cpp | 35 ++++++++++++++++++++++++ 6 files changed, 149 insertions(+) create mode 100644 folly/detail/ThreadLocalDetail.cpp create mode 100644 folly/test/ThreadLocalTestLib.cpp diff --git a/folly/Makefile.am b/folly/Makefile.am index 66390932..ed749242 100644 --- 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 bdac6cc3..d0c37d9b 100644 --- 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 index 00000000..58156365 --- /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 + +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 da3ee073..c97083ff 100644 --- a/folly/detail/ThreadLocalDetail.h +++ b/folly/detail/ThreadLocalDetail.h @@ -163,6 +163,43 @@ struct ThreadEntry { constexpr uint32_t kEntryIDInvalid = std::numeric_limits::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 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 lg(mutex_); + + keys_.push_back(key); + } + + std::mutex mutex_; + std::vector 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 82e6eb4f..823c6cb5 100644 --- a/folly/test/ThreadLocalTest.cpp +++ b/folly/test/ThreadLocalTest.cpp @@ -16,6 +16,7 @@ #include +#include #include #include #include @@ -37,6 +38,7 @@ #include #include +#include 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 index 00000000..1a81528b --- /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 +#include + +#include + +class A { + public: + void use() const { + } +}; + +folly::ThreadLocal a; + +extern "C" { + +void useA() { + a->use(); +} + +} -- 2.34.1