From: Steve O'Brien <steveo@fb.com>
Date: Sun, 19 Jul 2015 16:22:34 +0000 (-0700)
Subject: folly Singleton: clear some state if creator function fails
X-Git-Tag: v0.51.0~3
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=ad7e7f72235d3803b0077c7acbd082c79eb99709;p=folly.git

folly Singleton: clear some state if creator function fails

Summary: The creator thread ID is saved to indicate the singleton is already being built (to help detect dependency cycles).  However if the creation function throws an error, that thread ID persists, and then if the same thread tries again to build the singleton it will be falsely detected as a dependency cycle.  This clears that state in the case of failure.

Reviewed By: @chipturner

Differential Revision: D2256441
---

diff --git a/folly/Singleton-inl.h b/folly/Singleton-inl.h
index ecfeda1a..6bc43ad2 100644
--- a/folly/Singleton-inl.h
+++ b/folly/Singleton-inl.h
@@ -139,11 +139,17 @@ void SingletonHolder<T>::createInstance() {
     return;
   }
 
+  SCOPE_EXIT {
+    // Clean up creator thread when complete, and also, in case of errors here,
+    // so that subsequent attempts don't think this is still in the process of
+    // being built.
+    creating_thread_ = std::thread::id();
+  };
+
   creating_thread_ = std::this_thread::get_id();
 
   RWSpinLock::ReadHolder rh(&vault_.stateMutex_);
   if (vault_.state_ == SingletonVault::SingletonVaultState::Quiescing) {
-    creating_thread_ = std::thread::id();
     return;
   }
 
@@ -184,7 +190,6 @@ void SingletonHolder<T>::createInstance() {
 
   instance_weak_ = instance_;
   instance_ptr_ = instance_.get();
-  creating_thread_ = std::thread::id();
   destroy_baton_ = std::move(destroy_baton);
   print_destructor_stack_trace_ = std::move(print_destructor_stack_trace);
 
diff --git a/folly/test/SingletonTest.cpp b/folly/test/SingletonTest.cpp
index b4ffdbf6..8505d068 100644
--- a/folly/test/SingletonTest.cpp
+++ b/folly/test/SingletonTest.cpp
@@ -414,6 +414,32 @@ TEST(Singleton, SingletonConcurrency) {
   EXPECT_EQ(vault.livingSingletonCount(), 1);
 }
 
+struct ErrorConstructor {
+  static size_t constructCount_;
+  ErrorConstructor() {
+    if ((constructCount_++) == 0) {
+      throw std::runtime_error("first time fails");
+    }
+  }
+};
+size_t ErrorConstructor::constructCount_(0);
+
+struct CreationErrorTag {};
+template <typename T, typename Tag = detail::DefaultTag>
+using SingletonCreationError = Singleton<T, Tag, CreationErrorTag>;
+
+TEST(Singleton, SingletonCreationError) {
+  auto& vault = *SingletonVault::singleton<CreationErrorTag>();
+  SingletonCreationError<ErrorConstructor> error_once_singleton;
+
+  // first time should error out
+  EXPECT_THROW(error_once_singleton.get_weak().lock(), std::runtime_error);
+
+  // second time it'll work fine
+  error_once_singleton.get_weak().lock();
+  SUCCEED();
+}
+
 struct ConcurrencyStressTag {};
 template <typename T, typename Tag = detail::DefaultTag>
 using SingletonConcurrencyStress = Singleton <T, Tag, ConcurrencyStressTag>;