Make SingletonVault state use ReadPriority mutex

Summary: This fixes a deadlock possible when singleton chain is created concurrently with destroyInstances().

Reviewed By: lbrandy, yfeldblum

Differential Revision: D4329028

fbshipit-source-id: a11b3ff42d164ead2f8e3e77e0e17be43a8ad306

diff --git a/folly/Singleton.h b/folly/Singleton.h
index 9c575ed7cc6d81a12e0924657684ccfeaf776c24..1eb52f64068096c225852d9a75137161de3ced7c 100644 (file)
--- a/folly/Singleton.h
+++ b/folly/Singleton.h
@@ -507,7 +507,10 @@ class SingletonVault {
       eagerInitSingletons_;
   folly::Synchronized<std::vector<detail::TypeDescriptor>> creationOrder_;
 
-  folly::Synchronized<State> state_;
+  // Using SharedMutexReadPriority is important here, because we want to make
+  // sure we don't block nested singleton creation happening concurrently with
+  // destroyInstances().
+  folly::Synchronized<State, folly::SharedMutexReadPriority> state_;
 
   Type type_;
 };

diff --git a/folly/test/SingletonTest.cpp b/folly/test/SingletonTest.cpp
index 14c4efa621c5f371bdfb7cdf45dd82c937ea7a18..feff41e30292998a6be27c4c4f040eff5a96c21c 100644 (file)
--- a/folly/test/SingletonTest.cpp
+++ b/folly/test/SingletonTest.cpp
@@ -641,3 +641,36 @@ TEST(Singleton, CustomCreator) {
   EXPECT_EQ(42, x2p->a1);
   EXPECT_EQ(std::string("foo"), x2p->a2);
 }
+
+struct ConcurrentCreationDestructionTag {};
+template <typename T, typename Tag = detail::DefaultTag>
+using SingletonConcurrentCreationDestruction =
+    Singleton<T, Tag, ConcurrentCreationDestructionTag>;
+
+folly::Baton<> slowpokeNeedySingletonBaton;
+
+struct SlowpokeNeedySingleton {
+  SlowpokeNeedySingleton() {
+    slowpokeNeedySingletonBaton.post();
+    /* sleep override */ std::this_thread::sleep_for(
+        std::chrono::milliseconds(100));
+    auto unused =
+        SingletonConcurrentCreationDestruction<NeededSingleton>::try_get();
+    EXPECT_NE(unused, nullptr);
+  }
+};
+
+TEST(Singleton, ConcurrentCreationDestruction) {
+  auto& vault = *SingletonVault::singleton<ConcurrentCreationDestructionTag>();
+  SingletonConcurrentCreationDestruction<NeededSingleton> neededSingleton;
+  SingletonConcurrentCreationDestruction<SlowpokeNeedySingleton> needySingleton;
+  vault.registrationComplete();
+
+  std::thread needyThread([&] { needySingleton.try_get(); });
+
+  slowpokeNeedySingletonBaton.wait();
+
+  vault.destroyInstances();
+
+  needyThread.join();
+}