Commit 4140f483 authored by Steve O'Brien's avatar Steve O'Brien Committed by facebook-github-bot-0

folly: ThreadLocalDetail: make PthreadKeyUnregister constexpr-constructible, avoid SIOF

Summary:
Since this is used in so many places during the program's static-initialization (at startup), this class itself could be (has been demonstrated to be) a point of SIOF problems itself.

Made this class constexpr-constructible, so it doesn't need to be part of static initialization, making it SIOF-proof.

Reviewed By: luciang

Differential Revision: D2709231

fb-gh-sync-id: f248c9f2848c09045e000cfdc03636d847e522c9
parent 266ccb57
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
namespace folly { namespace threadlocal_detail { namespace folly { namespace threadlocal_detail {
PthreadKeyUnregister PthreadKeyUnregister PthreadKeyUnregister::instance_;
MAX_STATIC_CONSTRUCTOR_PRIORITY PthreadKeyUnregister::instance_;
}} }}
...@@ -163,20 +163,29 @@ struct ThreadEntry { ...@@ -163,20 +163,29 @@ struct ThreadEntry {
constexpr uint32_t kEntryIDInvalid = std::numeric_limits<uint32_t>::max(); constexpr uint32_t kEntryIDInvalid = std::numeric_limits<uint32_t>::max();
struct PthreadKeyUnregisterTester;
/** /**
* We want to disable onThreadExit call at the end of shutdown, we don't care * We want to disable onThreadExit call at the end of shutdown, we don't care
* about leaking memory at that point. * about leaking memory at that point.
* *
* Otherwise if ThreadLocal is used in a shared library, onThreadExit may be * Otherwise if ThreadLocal is used in a shared library, onThreadExit may be
* called after dlclose(). * called after dlclose().
*
* This class has one single static instance; however since it's so widely used,
* directly or indirectly, by so many classes, we need to take care to avoid
* problems stemming from the Static Initialization/Destruction Order Fiascos.
* Therefore this class needs to be constexpr-constructible, so as to avoid
* the need for this to participate in init/destruction order.
*/ */
class PthreadKeyUnregister { class PthreadKeyUnregister {
public: public:
static constexpr size_t kMaxKeys = 1UL << 16;
~PthreadKeyUnregister() { ~PthreadKeyUnregister() {
std::lock_guard<std::mutex> lg(mutex_); std::lock_guard<std::mutex> lg(mutex_);
while (size_--) {
for (const auto& key: keys_) { pthread_key_delete(keys_[size_]);
pthread_key_delete(key);
} }
} }
...@@ -185,21 +194,27 @@ class PthreadKeyUnregister { ...@@ -185,21 +194,27 @@ class PthreadKeyUnregister {
} }
private: private:
PthreadKeyUnregister() {} /**
* Only one global instance should exist, hence this is private.
* See also the important note at the top of this class about `constexpr`
* usage.
*/
constexpr PthreadKeyUnregister() : mutex_(), size_(0), keys_() { }
friend class folly::threadlocal_detail::PthreadKeyUnregisterTester;
void registerKeyImpl(pthread_key_t key) { void registerKeyImpl(pthread_key_t key) {
std::lock_guard<std::mutex> lg(mutex_); std::lock_guard<std::mutex> lg(mutex_);
CHECK_LT(size_, kMaxKeys);
keys_.push_back(key); keys_[size_++] = key;
} }
std::mutex mutex_; std::mutex mutex_;
std::vector<pthread_key_t> keys_; size_t size_;
pthread_key_t keys_[kMaxKeys];
static PthreadKeyUnregister instance_; static PthreadKeyUnregister instance_;
}; };
// Held in a singleton to track our global instances. // Held in a singleton to track our global instances.
// We have one of these per "Tag", by default one for the whole system // We have one of these per "Tag", by default one for the whole system
// (Tag=void). // (Tag=void).
......
...@@ -583,6 +583,19 @@ TEST(ThreadLocal, SharedLibrary) { ...@@ -583,6 +583,19 @@ TEST(ThreadLocal, SharedLibrary) {
t2.join(); t2.join();
} }
namespace folly { namespace threadlocal_detail {
struct PthreadKeyUnregisterTester {
PthreadKeyUnregister p;
constexpr PthreadKeyUnregisterTester() = default;
};
}}
TEST(ThreadLocal, UnregisterClassHasConstExprCtor) {
folly::threadlocal_detail::PthreadKeyUnregisterTester x;
// yep!
SUCCEED();
}
// clang is unable to compile this code unless in c++14 mode. // clang is unable to compile this code unless in c++14 mode.
#if __cplusplus >= 201402L #if __cplusplus >= 201402L
namespace { namespace {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment