Commit 272a2e9a authored by Kenny Yu's avatar Kenny Yu Committed by Facebook Github Bot

Fix TSAN lock inversion in VirtualEventBase and FiberManagerMap

Summary:
There is a lock inversion between VirtualEventBase and the fibers library:

1. During program shutdown, `GlobalCache::getImpl` acquires its `mutex_`, then it calls `VirtualEventBase::runOnDestruction` which acquires the SharedMutex on `onDestructionCallbacks_`
2. During normal program execution, `VirtualEventBase::destroyImpl()` acquires the `onDestructionCallbacks_` lock and then invokes the user-supplied callbacks, which will invoke `GlobalCache::eraseImpl` and acquire `mutex_`

The reason for this lock inversion is because in step 2, we are holding a mutex while invoking user-supplied callbacks.
The fix is to NOT hold the lock while we invoke these callbacks.

Reviewed By: andriigrynenko

Differential Revision: D9797816

fbshipit-source-id: 861a915f22138f0c8d3f8bca4d7bf4b9a0aa3d26
parent b7add7bc
......@@ -37,13 +37,15 @@ void VirtualEventBase::destroyImpl() {
clearCobTimeouts();
onDestructionCallbacks_.withWLock([&](LoopCallbackList& callbacks) {
while (!callbacks.empty()) {
auto& callback = callbacks.front();
callbacks.pop_front();
callback.runLoopCallback();
}
});
// To avoid potential deadlock, do not hold the mutex while invoking
// user-supplied callbacks.
LoopCallbackList callbacks;
onDestructionCallbacks_.swap(callbacks);
while (!callbacks.empty()) {
auto& callback = callbacks.front();
callbacks.pop_front();
callback.runLoopCallback();
}
}
destroyPromise_.set_value();
......
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