From: Andrii Grynenko <andrii@fb.com>
Date: Thu, 15 Dec 2016 04:02:46 +0000 (-0800)
Subject: Make SingletonVault state use ReadPriority mutex
X-Git-Tag: v2016.12.19.00~18
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=4092256e604e1cd140b991edeb0eb86c36ded56e;p=folly.git

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 9c575ed7..1eb52f64 100644
--- 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 14c4efa6..feff41e3 100644
--- 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();
+}