From: Andrii Grynenko Date: Fri, 26 Feb 2016 18:28:31 +0000 (-0800) Subject: Fix folly::ThreadLocal to have unique singleton in dev builds X-Git-Tag: deprecate-dynamic-initializer~34 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=a7d7c07cadb5d5e9d55d5dd97b3e6069afe21245;p=folly.git Fix folly::ThreadLocal to have unique singleton in dev builds Summary:This re-uses StaticSingletonManager which was previously used to fix the same issue in folly::Singleton. Because of the same issue we can no longer use static thread_local for the ThreadEntry. We now rely on pthread_getspecific/pthread_setspecific instead and use static thread_local ThreadEntry* only as a cache (to improve perf). Reviewed By: yfeldblum Differential Revision: D2978526 fb-gh-sync-id: cf1d9044afc27b62bd50a1ed931c0c420ae7107e shipit-source-id: cf1d9044afc27b62bd50a1ed931c0c420ae7107e --- diff --git a/folly/Makefile.am b/folly/Makefile.am index f46b13ad..e910a2cb 100644 --- a/folly/Makefile.am +++ b/folly/Makefile.am @@ -70,6 +70,7 @@ nobase_follyinclude_HEADERS = \ detail/Sleeper.h \ detail/SlowFingerprint.h \ detail/SpinLockImpl.h \ + detail/StaticSingletonManager.h \ detail/Stats.h \ detail/ThreadLocalDetail.h \ detail/TurnSequencer.h \ @@ -360,6 +361,7 @@ libfolly_la_SOURCES = \ futures/QueuedImmediateExecutor.cpp \ futures/ThreadWheelTimekeeper.cpp \ detail/Futex.cpp \ + detail/StaticSingletonManager.cpp \ detail/ThreadLocalDetail.cpp \ GroupVarint.cpp \ GroupVarintTables.cpp \ diff --git a/folly/Singleton.cpp b/folly/Singleton.cpp index cf1ce727..8e9cd99b 100644 --- a/folly/Singleton.cpp +++ b/folly/Singleton.cpp @@ -25,12 +25,6 @@ namespace folly { namespace detail { -// This implementation should always live in .cpp file. -StaticSingletonManager& StaticSingletonManager::instance() { - static StaticSingletonManager* instance = new StaticSingletonManager(); - return *instance; -} - constexpr std::chrono::seconds SingletonHolderBase::kDestroyWaitTime; } diff --git a/folly/Singleton.h b/folly/Singleton.h index bb04862d..c5fe79db 100644 --- a/folly/Singleton.h +++ b/folly/Singleton.h @@ -110,6 +110,7 @@ #include #include #include +#include #include #include @@ -160,62 +161,6 @@ class SingletonVault; namespace detail { -// This internal-use-only class is used to create all leaked Meyers singletons. -// It guarantees that only one instance of every such singleton will ever be -// created, even when requested from different compilation units linked -// dynamically. -class StaticSingletonManager { - public: - static StaticSingletonManager& instance(); - - template - inline T* create(F&& creator) { - auto& entry = [&]() mutable -> Entry& { - std::lock_guard lg(mutex_); - - auto& id = typeid(TypePair); - auto& entryPtr = reinterpret_cast*&>(map_[id]); - if (!entryPtr) { - entryPtr = new Entry(); - } - return *entryPtr; - }(); - - std::lock_guard lg(entry.mutex); - - if (!entry.ptr) { - entry.ptr = creator(); - } - return entry.ptr; - } - - private: - template - class TypePair {}; - - StaticSingletonManager() {} - - template - struct Entry { - T* ptr{nullptr}; - std::mutex mutex; - }; - - std::unordered_map map_; - std::mutex mutex_; -}; - -template -inline T* createGlobal(F&& creator) { - return StaticSingletonManager::instance().create( - std::forward(creator)); -} - -template -inline T* createGlobal() { - return createGlobal([]() { return new T(); }); -} - struct DefaultTag {}; // A TypeDescriptor is the unique handle for a given singleton. It is diff --git a/folly/detail/StaticSingletonManager.cpp b/folly/detail/StaticSingletonManager.cpp new file mode 100644 index 00000000..23464cc1 --- /dev/null +++ b/folly/detail/StaticSingletonManager.cpp @@ -0,0 +1,27 @@ +/* + * Copyright 2016 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 detail { + +// This implementation should always live in .cpp file. +StaticSingletonManager& StaticSingletonManager::instance() { + static StaticSingletonManager* instance = new StaticSingletonManager(); + return *instance; +} +} +} diff --git a/folly/detail/StaticSingletonManager.h b/folly/detail/StaticSingletonManager.h new file mode 100644 index 00000000..6e4b63d6 --- /dev/null +++ b/folly/detail/StaticSingletonManager.h @@ -0,0 +1,87 @@ +/* + * Copyright 2016 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. + */ + +#pragma once + +#include +#include +#include + +namespace folly { +namespace detail { + +// This internal-use-only class is used to create all leaked Meyers singletons. +// It guarantees that only one instance of every such singleton will ever be +// created, even when requested from different compilation units linked +// dynamically. +class StaticSingletonManager { + public: + static StaticSingletonManager& instance(); + + template + inline T* create(F&& creator) { + auto& entry = [&]() mutable -> Entry& { + std::lock_guard lg(mutex_); + + auto& id = typeid(TypePair); + auto& entryPtr = map_[id]; + if (!entryPtr) { + entryPtr = new Entry(); + } + assert(dynamic_cast*>(entryPtr) != nullptr); + return *static_cast*>(entryPtr); + }(); + + std::lock_guard lg(entry.mutex); + + if (!entry.ptr) { + entry.ptr = creator(); + } + return entry.ptr; + } + + private: + template + class TypePair {}; + + StaticSingletonManager() {} + + struct EntryIf { + virtual ~EntryIf() {} + }; + + template + struct Entry : public EntryIf { + T* ptr{nullptr}; + std::mutex mutex; + }; + + std::unordered_map map_; + std::mutex mutex_; +}; + +template +inline T* createGlobal(F&& creator) { + return StaticSingletonManager::instance().create( + std::forward(creator)); +} + +template +inline T* createGlobal() { + return createGlobal([]() { return new T(); }); +} +} +} diff --git a/folly/detail/ThreadLocalDetail.h b/folly/detail/ThreadLocalDetail.h index 90e57849..e9843269 100644 --- a/folly/detail/ThreadLocalDetail.h +++ b/folly/detail/ThreadLocalDetail.h @@ -31,6 +31,8 @@ #include #include +#include + // In general, emutls cleanup is not guaranteed to play nice with the way // StaticMeta mixes direct pthread calls and the use of __thread. This has // caused problems on multiple platforms so don't use __thread there. @@ -156,10 +158,10 @@ struct ElementWrapper { * (under the lock). */ struct ThreadEntry { - ElementWrapper* elements; - size_t elementsCapacity; - ThreadEntry* next; - ThreadEntry* prev; + ElementWrapper* elements{nullptr}; + size_t elementsCapacity{0}; + ThreadEntry* next{nullptr}; + ThreadEntry* prev{nullptr}; }; constexpr uint32_t kEntryIDInvalid = std::numeric_limits::max(); @@ -273,9 +275,8 @@ struct StaticMeta { static StaticMeta& instance() { // Leak it on exit, there's only one per process and we don't have to // worry about synchronization with exiting threads. - static bool constructed = (inst_ = new StaticMeta()); - (void)constructed; // suppress unused warning - return *inst_; + static auto instance = detail::createGlobal, void>(); + return *instance; } uint32_t nextId_; @@ -297,11 +298,6 @@ struct StaticMeta { t->next = t->prev = t; } -#ifdef FOLLY_TLD_USE_FOLLY_TLS - static FOLLY_TLS ThreadEntry threadEntry_; -#endif - static StaticMeta* inst_; - StaticMeta() : nextId_(1) { head_.next = head_.prev = &head_; int ret = pthread_key_create(&pthreadKey_, &onThreadExit); @@ -326,10 +322,7 @@ struct StaticMeta { LOG(FATAL) << "StaticMeta lives forever!"; } - static ThreadEntry* getThreadEntry() { -#ifdef FOLLY_TLD_USE_FOLLY_TLS - return &threadEntry_; -#else + static ThreadEntry* getThreadEntrySlow() { auto key = instance().pthreadKey_; ThreadEntry* threadEntry = static_cast(pthread_getspecific(key)); @@ -339,6 +332,17 @@ struct StaticMeta { checkPosixError(ret, "pthread_setspecific failed"); } return threadEntry; + } + + static ThreadEntry* getThreadEntry() { +#ifdef FOLLY_TLD_USE_FOLLY_TLS + static FOLLY_TLS ThreadEntry* threadEntryCache{nullptr}; + if (UNLIKELY(threadEntryCache == nullptr)) { + threadEntryCache = getThreadEntrySlow(); + } + return threadEntryCache; +#else + return getThreadEntrySlow(); #endif } @@ -346,37 +350,32 @@ struct StaticMeta { instance().lock_.lock(); // Make sure it's created } - static void onForkParent(void) { - inst_->lock_.unlock(); - } + static void onForkParent(void) { instance().lock_.unlock(); } static void onForkChild(void) { // only the current thread survives - inst_->head_.next = inst_->head_.prev = &inst_->head_; + instance().head_.next = instance().head_.prev = &instance().head_; ThreadEntry* threadEntry = getThreadEntry(); // If this thread was in the list before the fork, add it back. if (threadEntry->elementsCapacity != 0) { - inst_->push_back(threadEntry); + instance().push_back(threadEntry); } - inst_->lock_.unlock(); + instance().lock_.unlock(); } static void onThreadExit(void* ptr) { auto& meta = instance(); -#ifdef FOLLY_TLD_USE_FOLLY_TLS - ThreadEntry* threadEntry = getThreadEntry(); - DCHECK_EQ(ptr, &meta); - DCHECK_GT(threadEntry->elementsCapacity, 0); -#else // pthread sets the thread-specific value corresponding // to meta.pthreadKey_ to NULL before calling onThreadExit. // We need to set it back to ptr to enable the correct behaviour // of the subsequent calls of getThreadEntry // (which may happen in user-provided custom deleters) pthread_setspecific(meta.pthreadKey_, ptr); - ThreadEntry* threadEntry = static_cast(ptr); -#endif + + ThreadEntry* threadEntry = getThreadEntry(); + DCHECK_GT(threadEntry->elementsCapacity, 0); + { std::lock_guard g(meta.lock_); meta.erase(threadEntry); @@ -399,10 +398,7 @@ struct StaticMeta { threadEntry->elements = nullptr; pthread_setspecific(meta.pthreadKey_, nullptr); -#ifndef FOLLY_TLD_USE_FOLLY_TLS - // Allocated in getThreadEntry() when not using folly TLS; free it delete threadEntry; -#endif } static uint32_t allocate(EntryID* ent) { @@ -558,12 +554,6 @@ struct StaticMeta { } free(reallocated); - -#ifdef FOLLY_TLD_USE_FOLLY_TLS - if (prevCapacity == 0) { - pthread_setspecific(meta.pthreadKey_, &meta); - } -#endif } static ElementWrapper& get(EntryID* ent) { @@ -580,13 +570,6 @@ struct StaticMeta { } }; -#ifdef FOLLY_TLD_USE_FOLLY_TLS -template -FOLLY_TLS ThreadEntry StaticMeta::threadEntry_ = {nullptr, 0, - nullptr, nullptr}; -#endif -template StaticMeta* StaticMeta::inst_ = nullptr; - } // namespace threadlocal_detail } // namespace folly