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

Suppress noisy TSAN lock inversions in folly::Singleton

Summary:
When running tests with a TSAN-annotated version of folly::SharedMutex,
I discovered at least 3 types of lock inversions in folly::Singleton code.
These inversions are probably benign and deadlock is not possible, but we need to
suppress these inversions to prevent too much noise from TSAN.

Lock inversions:

1. The users can supply arbitrary create() functions for Singletons. If the create() function
   happens to acquire a mutex, then this introduces a lock ordering from
   SingletonVault -> mutex. Later on, when the user code runs in normal operations, we might
   hold the mutex and try to create a singleton, resulting in the mutex -> SingletonVault ordering
2. SingletonVault::destroyInstances() holds a mutex, and then invokes the user-supplied destroy()
   functions. In the user supplied destroy functions, we might attempt to create a singleton and
   acquire the lock in SingletonHolder, resulting in SingletonVault -> SingletonHolder lock ordering.
   In normal operations, whenever a singleton is created, we have the SingletonHolder -> SingletonVault
   lock ordering in createInstance()
3. doEagerInit() holds the eagerInitSingleton mutex and then acquires the SingletonVault lock
   during createInstance(). addEagerInitSingleton() holds the SingletonVault lock and then acquires
   the eagerInitSingleton lock.

The source of these errors is that we are invoking user-supplied callbacks while holding mutexes.
However, these lock inversions cannot actually deadlock based on higher-level knowledge of folly::Singleton
state transitions.

To suppress these errors, use the new `folly::SharedMutexSuppressTSAN` mutex instead, which will
not be annotated by TSAN. If we are not building with TSAN, then this mutex is equivalent to a normal
`folly::SharedMutex`.

Reviewed By: andriigrynenko

Differential Revision: D9797988

fbshipit-source-id: 82b5850fe189f7d1dcaca0e3562fabcfafd0cef5
parent 2924934a
...@@ -539,10 +539,17 @@ class SingletonVault { ...@@ -539,10 +539,17 @@ class SingletonVault {
detail::SingletonHolderBase*, detail::SingletonHolderBase*,
detail::TypeDescriptorHasher> detail::TypeDescriptorHasher>
SingletonMap; SingletonMap;
Synchronized<SingletonMap> singletons_;
Synchronized<std::unordered_set<detail::SingletonHolderBase*>> // Use SharedMutexSuppressTSAN to suppress noisy lock inversions when building
// with TSAN. If TSAN is not enabled, SharedMutexSuppressTSAN is equivalent
// to a normal SharedMutex.
Synchronized<SingletonMap, SharedMutexSuppressTSAN> singletons_;
Synchronized<
std::unordered_set<detail::SingletonHolderBase*>,
SharedMutexSuppressTSAN>
eagerInitSingletons_; eagerInitSingletons_;
Synchronized<std::vector<detail::TypeDescriptor>> creationOrder_; Synchronized<std::vector<detail::TypeDescriptor>, SharedMutexSuppressTSAN>
creationOrder_;
// Using SharedMutexReadPriority is important here, because we want to make // Using SharedMutexReadPriority is important here, because we want to make
// sure we don't block nested singleton creation happening concurrently with // sure we don't block nested singleton creation happening concurrently with
......
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