Commit 9a9dfd3b authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot

Fix SingletonRelaxedCounter in T::~T() of ThreadLocal<T>

Summary:
[Folly] Fix `SingletonRelaxedCounter` in `T::~T()` of `ThreadLocal<T>`.

Given a type `T` which has a dtor which increments some `SingletonRelaxedCounter`, and given some use of a `ThreadLocal<T>` where, within some given thread, the only time `SingletonRelaexCounter` is incremented is in the `T` dtor, there would be a leak of the `LocalLifetime` and this leak would cause subsequent `SingletonRelaxedCounter::count()` to segfault with access to a deallocated local counter.

C++ `thread_local` destructors run before `pthread` thread-specific destructors. If a `pthread` thread-specific destructor triggers initialization of a C++ `thread_local`, then that `thread_local` will be leaked in that its destructor will never be run.

Fix this with `ThreadLocal` by detecting when `ThreadLocal` per-thread destructors have begun running. This is a larger issue with `pthread` thread-specific objects overall, but as long as we stick to using `ThreadLocal`, it can be papered over.

Reviewed By: davidtgoldblatt

Differential Revision: D13970469

fbshipit-source-id: a4c7f36d2c0d63f8f0e363ddb9d35a44b027aea6
parent 0779c7fa
...@@ -108,6 +108,15 @@ ThreadEntryList* StaticMetaBase::getThreadEntryList() { ...@@ -108,6 +108,15 @@ ThreadEntryList* StaticMetaBase::getThreadEntryList() {
#endif #endif
} }
bool StaticMetaBase::dying() {
for (auto te = getThreadEntryList()->head; te; te = te->listNext) {
if (te->removed_) {
return true;
}
}
return false;
}
void StaticMetaBase::onThreadExit(void* ptr) { void StaticMetaBase::onThreadExit(void* ptr) {
auto threadEntry = static_cast<ThreadEntry*>(ptr); auto threadEntry = static_cast<ThreadEntry*>(ptr);
......
...@@ -356,6 +356,8 @@ struct StaticMetaBase { ...@@ -356,6 +356,8 @@ struct StaticMetaBase {
FOLLY_EXPORT static ThreadEntryList* getThreadEntryList(); FOLLY_EXPORT static ThreadEntryList* getThreadEntryList();
static bool dying();
static void onThreadExit(void* ptr); static void onThreadExit(void* ptr);
// returns the elementsCapacity for the // returns the elementsCapacity for the
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include <folly/Synchronized.h> #include <folly/Synchronized.h>
#include <folly/Utility.h> #include <folly/Utility.h>
#include <folly/detail/StaticSingletonManager.h> #include <folly/detail/StaticSingletonManager.h>
#include <folly/detail/ThreadLocalDetail.h>
namespace folly { namespace folly {
...@@ -169,6 +170,9 @@ class SingletonRelaxedCounter { ...@@ -169,6 +170,9 @@ class SingletonRelaxedCounter {
} }
FOLLY_NOINLINE static Counter* counterSlow(CounterAndCache& state) { FOLLY_NOINLINE static Counter* counterSlow(CounterAndCache& state) {
if (threadlocal_detail::StaticMetaBase::dying()) {
return &Global::instance().fallback;
}
lifetime().track(state); // idempotent lifetime().track(state); // idempotent
auto const cache = state.cache; auto const cache = state.cache;
return FOLLY_LIKELY(!!cache) ? cache : &Global::instance().fallback; return FOLLY_LIKELY(!!cache) ? cache : &Global::instance().fallback;
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include <boost/thread/barrier.hpp> #include <boost/thread/barrier.hpp>
#include <folly/ThreadLocal.h>
#include <folly/portability/GTest.h> #include <folly/portability/GTest.h>
namespace folly { namespace folly {
...@@ -41,6 +42,39 @@ TEST_F(SingletonRelaxedCounterTest, Basic) { ...@@ -41,6 +42,39 @@ TEST_F(SingletonRelaxedCounterTest, Basic) {
EXPECT_EQ(0, Counter::count()); EXPECT_EQ(0, Counter::count());
} }
TEST_F(SingletonRelaxedCounterTest, CompatibilityWithThreadLocal) {
struct CounterNoOpInDtor {
~CounterNoOpInDtor() {
Counter::add(0);
}
};
ThreadLocal<CounterNoOpInDtor> tl;
auto thread = std::thread([&] { //
std::ignore = *tl;
});
thread.join();
EXPECT_EQ(0, Counter::count());
}
TEST_F(SingletonRelaxedCounterTest, CompatibilityWithThreadLocalMany) {
struct CounterNoOpInDtor {
~CounterNoOpInDtor() {
Counter::add(0);
}
};
ThreadLocal<CounterNoOpInDtor> tl;
std::vector<std::thread> threads(16);
for (auto& thread : threads) {
thread = std::thread([&] { //
std::ignore = *tl;
});
}
for (auto& thread : threads) {
thread.join();
}
EXPECT_EQ(0, Counter::count());
}
TEST_F(SingletonRelaxedCounterTest, MultithreadCorrectness) { TEST_F(SingletonRelaxedCounterTest, MultithreadCorrectness) {
static constexpr size_t const kPerThreadIncrements = 1000000; static constexpr size_t const kPerThreadIncrements = 1000000;
static constexpr size_t const kNumThreads = 24; static constexpr size_t const kNumThreads = 24;
......
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