Skip to content

Commit

Permalink
Fix TSAN deadlock in ThreadLocal's fork handler.
Browse files Browse the repository at this point in the history
Summary:
The per ThreadEntrySet lock causes the number of locks to be tracked
by TSAN during fork to be too large. We cannot control order of fork handler
registration and it seems like TSAN's handlers are run earlier and lock down
some of its internal state. Later when a realloc triggered in TSAN's tracking
logic when we acquire all the ThreadEntrySet lock causes the fork to deadlock.

For now, we can simply skip locking the per ThreadEntrySet lock in fork
handlers. reset() and accessAllThreads() path are the only ones that end up
holding locks on them and both paths also hold the accessAllThreadsLock_ as
well and that is accounted for in the fork handler.

Reviewed By: chippermist

Differential Revision: D58920872

fbshipit-source-id: dbdc966f2586bb5479e0bf3274e54d3e9e5a2c6c
  • Loading branch information
Nitin Garg authored and facebook-github-bot committed Jun 24, 2024
1 parent 216baa6 commit 59620c8
Showing 1 changed file with 10 additions and 17 deletions.
27 changes: 10 additions & 17 deletions folly/detail/ThreadLocalDetail.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,19 +652,14 @@ struct FOLLY_EXPORT StaticMeta final : StaticMetaBase {
return false;
}
meta.lock_.lock();
size_t maxId = meta.allId2ThreadEntrySets_.size();
for (size_t id = 0; id < maxId; ++id) {
meta.allId2ThreadEntrySets_[id].unsafeGetMutex().lock();
}
// Okay to not lock each set in meta.allId2ThreadEntrySets
// as accessAllThreadsLock_ in held by calls to reset() and
// accessAllThreads.
return true;
}

static void onForkParent() {
auto& meta = instance();
size_t maxId = meta.allId2ThreadEntrySets_.size();
for (size_t id = 0; id < maxId; ++id) {
meta.allId2ThreadEntrySets_[id].unsafeGetMutex().unlock();
}
meta.lock_.unlock();
meta.accessAllThreadsLock_.unlock();
}
Expand All @@ -676,24 +671,22 @@ struct FOLLY_EXPORT StaticMeta final : StaticMetaBase {
// Loop through allId2ThreadEntrySets_; Only keep ThreadEntry* in the map
// for ThreadEntry::elements that are still in use by the current thread.
// Evict all of the ThreadEntry* from other threads.
size_t maxId = meta.allId2ThreadEntrySets_.size();
for (size_t id = 0; id < maxId; ++id) {
auto& teSet = meta.allId2ThreadEntrySets_[id];
auto& wlockedSet = teSet.unsafeGetUnlocked(); // locked in preFork
if (wlockedSet.contains(threadEntry)) {
uint32_t maxId = meta.nextId_.load();
for (uint32_t id = 0; id < maxId; ++id) {
auto wlockedSet = meta.allId2ThreadEntrySets_[id].wlock();
if (wlockedSet->contains(threadEntry)) {
DCHECK(threadEntry->elements[id].isLinked);
wlockedSet.clear();
wlockedSet.insert(threadEntry);
wlockedSet->clear();
wlockedSet->insert(threadEntry);
} else {
// DCHECK if elements[id] is unlinked for this
// threadEntry only (id < threadEntry->getElementsCapacity()).
// The rest can be cleared regardless.
if (id < threadEntry->getElementsCapacity()) {
DCHECK(!threadEntry->elements[id].isLinked);
}
wlockedSet.clear();
wlockedSet->clear();
}
teSet.unsafeGetMutex().unlock();
}
meta.lock_.unlock();
meta.accessAllThreadsLock_.unlock();
Expand Down

0 comments on commit 59620c8

Please sign in to comment.