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

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);
 

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>;