From 6caa69566e8e747ffcdb5fb5499315b0e5703ef7 Mon Sep 17 00:00:00 2001
From: Steve O'Brien <steveo@fb.com>
Date: Tue, 7 Jul 2015 16:10:07 -0700
Subject: [PATCH] folly/singleton: fatal in unrecoverable error cases

Summary: Early in the startup process there may not be a default signal handler installed, and stack traces are not available; also during the startup process is when init-order fiascos occur.  Dump a stacktrace and fatal when an unregistered singleton is used.

Also fatals -- with glog `LOG(FATAL)`, which triggers an ABRT signal -- on other illegal and unrecoverable usage like double-registration or circular dependency.

Reviewed By: @andriigrynenko

Differential Revision: D2200408
---
 folly/Singleton-inl.h         | 14 +++++++++-----
 folly/SingletonStackTrace.cpp |  6 +++++-
 folly/test/SingletonTest.cpp  | 28 +++++++++++-----------------
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/folly/Singleton-inl.h b/folly/Singleton-inl.h
index c853da65..ecfeda1a 100644
--- a/folly/Singleton-inl.h
+++ b/folly/Singleton-inl.h
@@ -32,7 +32,7 @@ void SingletonHolder<T>::registerSingleton(CreateFunc c, TeardownFunc t) {
   std::lock_guard<std::mutex> entry_lock(mutex_);
 
   if (state_ != SingletonHolderState::NotRegistered) {
-    throw std::logic_error("Double registration");
+    LOG(FATAL) << "Double registration of singleton: " << type_.name();
   }
 
   create_ = std::move(c);
@@ -44,7 +44,8 @@ void SingletonHolder<T>::registerSingleton(CreateFunc c, TeardownFunc t) {
 template <typename T>
 void SingletonHolder<T>::registerSingletonMock(CreateFunc c, TeardownFunc t) {
   if (state_ == SingletonHolderState::NotRegistered) {
-    throw std::logic_error("Registering mock before singleton was registered");
+    LOG(FATAL)
+        << "Registering mock before singleton was registered: " << type_.name();
   }
   destroyInstance();
 
@@ -119,8 +120,7 @@ void SingletonHolder<T>::createInstance() {
   // for creating_thread if it was set by other thread, but we only care about
   // it if it was set by current thread anyways.
   if (creating_thread_ == std::this_thread::get_id()) {
-    throw std::out_of_range(std::string("circular singleton dependency: ") +
-                            type_.name());
+    LOG(FATAL) << "circular singleton dependency: " << type_.name();
   }
 
   std::lock_guard<std::mutex> entry_lock(mutex_);
@@ -128,7 +128,11 @@ void SingletonHolder<T>::createInstance() {
     return;
   }
   if (state_ == SingletonHolderState::NotRegistered) {
-    throw std::out_of_range("Creating instance for unregistered singleton");
+    auto ptr = SingletonVault::stackTraceGetter().load();
+    LOG(FATAL) << "Creating instance for unregistered singleton: "
+               << type_.name() << "\n"
+               << "Stacktrace:"
+               << "\n" << (ptr ? (*ptr)() : "(not available)");
   }
 
   if (state_ == SingletonHolderState::Living) {
diff --git a/folly/SingletonStackTrace.cpp b/folly/SingletonStackTrace.cpp
index 3e1f87cc..15ee94b4 100644
--- a/folly/SingletonStackTrace.cpp
+++ b/folly/SingletonStackTrace.cpp
@@ -46,8 +46,12 @@ struct SetStackTraceGetter {
   }
 };
 
+#ifdef __APPLE__
+// OS X doesn't support constructor priorities.
 SetStackTraceGetter setStackTraceGetter;
-
+#else
+SetStackTraceGetter __attribute__((__init_priority__(101))) setStackTraceGetter;
+#endif
 }
 
 }
diff --git a/folly/test/SingletonTest.cpp b/folly/test/SingletonTest.cpp
index f5d5983a..b4ffdbf6 100644
--- a/folly/test/SingletonTest.cpp
+++ b/folly/test/SingletonTest.cpp
@@ -79,8 +79,7 @@ TEST(Singleton, BasicGlobalUsage) {
 }
 
 TEST(Singleton, MissingSingleton) {
-  EXPECT_THROW([]() { auto u = Singleton<UnregisteredWatchdog>::get(); }(),
-               std::out_of_range);
+  EXPECT_DEATH([]() { auto u = Singleton<UnregisteredWatchdog>::get(); }(), "");
 }
 
 struct BasicUsageTag {};
@@ -204,26 +203,24 @@ TEST(Singleton, NaughtyUsage) {
   vault.registrationComplete();
 
   // Unregistered.
-  EXPECT_THROW(Singleton<Watchdog>::get(), std::out_of_range);
-  EXPECT_THROW(SingletonNaughtyUsage<Watchdog>::get(), std::out_of_range);
+  EXPECT_DEATH(Singleton<Watchdog>::get(), "");
+  EXPECT_DEATH(SingletonNaughtyUsage<Watchdog>::get(), "");
 
   vault.destroyInstances();
 
   auto& vault2 = *SingletonVault::singleton<NaughtyUsageTag2>();
 
-  EXPECT_THROW(SingletonNaughtyUsage2<Watchdog>::get(), std::logic_error);
+  EXPECT_DEATH(SingletonNaughtyUsage2<Watchdog>::get(), "");
   SingletonNaughtyUsage2<Watchdog> watchdog_singleton;
+
   // double registration
-  EXPECT_THROW([]() {
-      SingletonNaughtyUsage2<Watchdog> watchdog_singleton;
-    }(),
-    std::logic_error);
+  EXPECT_DEATH([]() { SingletonNaughtyUsage2<Watchdog> watchdog_singleton; }(),
+               "");
   vault2.destroyInstances();
+
   // double registration after destroy
-  EXPECT_THROW([]() {
-      SingletonNaughtyUsage2<Watchdog> watchdog_singleton;
-    }(),
-    std::logic_error);
+  EXPECT_DEATH([]() { SingletonNaughtyUsage2<Watchdog> watchdog_singleton; }(),
+               "");
 }
 
 struct SharedPtrUsageTag {};
@@ -374,10 +371,7 @@ TEST(Singleton, SingletonDependencies) {
   auto& self_needy_vault = *SingletonVault::singleton<SelfNeedyTag>();
 
   self_needy_vault.registrationComplete();
-  EXPECT_THROW([]() {
-      SingletonSelfNeedy<SelfNeedySingleton>::get();
-    }(),
-    std::out_of_range);
+  EXPECT_DEATH([]() { SingletonSelfNeedy<SelfNeedySingleton>::get(); }(), "");
 }
 
 // A test to ensure multiple threads contending on singleton creation
-- 
2.34.1