Commit 3cc2f315 authored by Kenny Yu's avatar Kenny Yu Committed by Facebook Github Bot

Make TSAN aware of unlock_shared() called on a different thread in ObserverManager

Summary:
`folly::SharedMutex` allows `unlock_shared()` to be called in a different thread from the one that locked it
There are some places in folly that use this property.

Example: In `ObserverManager`, the calling thread

1. acquires the read lock
2. then it schedules some async work and gives ownership of the lock guard to the lambda
3. the thread that performs the async work will unlock the mutex

However, this causes false positives for TSAN. It looks like this breaks TSAN's assumptions that a read-write mutex is always read locked and unlocked in the same thread.
In the example above, TSAN thinks the calling thread in step (1) still owns the mutex, even though semantically the mutex is now owned by the thread in step (3).

This results in false positives for running unit tests with an annotated version of `folly::SharedMutex`.
TSAN reports a lock inversion between the `folly::Singleton vaule_.state_` mutex, and the read lock referenced in `ObserverManager`.

To suppress this error, this change annotates ObserverManager to let TSAN know that the the thread in step 1 has dropped
the lock, and the thread in step 3 now owns the lock.

Reviewed By: andriigrynenko

Differential Revision: D9797937

fbshipit-source-id: c0e5277bb9cb2f404df8abd3c3d6ea4a16460c78
parent b2a33051
...@@ -78,11 +78,25 @@ class ObserverManager { ...@@ -78,11 +78,25 @@ class ObserverManager {
SharedMutexReadPriority::ReadHolder rh(instance->versionMutex_); SharedMutexReadPriority::ReadHolder rh(instance->versionMutex_);
instance->scheduleCurrent( // TSAN assumes that the thread that locks the mutex must
[core = std::move(core), // be the one that unlocks it. However, we are passing ownership of
instancePtr = instance.get(), // the read lock into the lambda, and the thread that performs the async
rh = std::move(rh), // work will be the one that unlocks it. To avoid noise with TSAN,
force]() { core->refresh(instancePtr->version_, force); }); // annotate that the thread has released the mutex, and then annotate
// the async thread as acquiring the mutex.
FOLLY_ANNOTATE_RWLOCK_RELEASED(
&instance->versionMutex_, FOLLY_ANNOTATE_RWLOCK_RDLOCK);
instance->scheduleCurrent([core = std::move(core),
instancePtr = instance.get(),
rh = std::move(rh),
force]() {
// Make TSAN know that the current thread owns the read lock now.
FOLLY_ANNOTATE_RWLOCK_ACQUIRED(
&instancePtr->versionMutex_, FOLLY_ANNOTATE_RWLOCK_RDLOCK);
core->refresh(instancePtr->version_, force);
});
} }
static void scheduleRefreshNewVersion(Core::WeakPtr coreWeak) { static void scheduleRefreshNewVersion(Core::WeakPtr coreWeak) {
......
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