Back out "Back out "Revert D61286579""

Summary:
Original commit changeset: 251ac011c75f

Original Phabricator Diff: D61629106

Reviewed By: dogdoggoos

Differential Revision: D61776376

fbshipit-source-id: aa8a17c55ccedd8e5c218ab19f985bace26fe2dd

folly/BUCK

@@ -1275,8 +1275,6 @@ cpp_library(
         "//folly/lang:safe_assert",
         "//folly/portability:config",
         "//folly/portability:fmt_compile",
-        "//folly/system:pid",
-        "//folly/system:thread_name",
     ],
     exported_deps = [
         ":cancellation_token",

folly/Singleton-inl.h

@@ -294,7 +294,6 @@ void SingletonHolder<T>::createInstance() {
       std::make_shared<std::atomic<bool>>(false);
 
   // Can't use make_shared -- no support for a custom deleter, sadly.
-  vault_.addToShutdownLog("Creating " + type().name());
   std::shared_ptr<T> instance(
       create_(),
       [destroy_baton, print_destructor_stack_trace, type = type()](T*) mutable {
@@ -303,7 +302,6 @@ void SingletonHolder<T>::createInstance() {
           detail::singletonPrintDestructionStackTrace(type);
         }
       });
-  vault_.addToShutdownLog(type().name() + " created.");
 
   // We should schedule destroyInstances() only after the singleton was
   // created. This will ensure it will be destroyed before singletons,

folly/Singleton.cpp

@@ -36,8 +36,6 @@
 #include <folly/lang/SafeAssert.h>
 #include <folly/portability/Config.h>
 #include <folly/portability/FmtCompile.h>
-#include <folly/system/Pid.h>
-#include <folly/system/ThreadName.h>
 // Before registrationComplete() we cannot assume that glog has been
 // initialized, so we need to use RAW_LOG for any message that may be logged
 // before that.
@@ -457,17 +455,12 @@ void SingletonVault::addToShutdownLog(std::string message) {
   std::chrono::milliseconds millis =
       std::chrono::duration_cast<std::chrono::milliseconds>(
           now.time_since_epoch());
-  shutdownLog_.wlock()->push_back(fmt::format(
-      "{:%T} {} on thread {}, pid {}",
-      millis,
-      message,
-      folly::getCurrentThreadName().value_or("{unset thread name}"),
-      folly::get_cached_pid()));
+  shutdownLog_.wlock()->push_back(fmt::format("{:%T} {}", millis, message));
 }
 
 #if FOLLY_HAVE_LIBRT
 namespace {
-void fireShutdownSignalHelper(sigval_t sigval) {
+[[noreturn]] void fireShutdownSignalHelper(sigval_t sigval) {
   static_cast<SingletonVault*>(sigval.sival_ptr)->fireShutdownTimer();
 }
 } // namespace
@@ -502,7 +495,7 @@ void SingletonVault::startShutdownTimer() {
 #endif
 }
 
-void SingletonVault::fireShutdownTimer() {
+[[noreturn]] void SingletonVault::fireShutdownTimer() {
   std::string shutdownLog;
   for (auto& logMessage : shutdownLog_.copy()) {
     shutdownLog += logMessage + "\n";
@@ -513,17 +506,7 @@ void SingletonVault::fireShutdownTimer() {
       std::chrono::milliseconds(shutdownTimeout_).count(),
       "ms. Shutdown log:\n",
       shutdownLog);
-  shutdownLogOutputHandler_(msg);
-}
-
-[[noreturn]] void SingletonVault::defaultShutdownLogOutputHandler(
-    std::string msg) {
   folly::terminate_with<std::runtime_error>(msg);
 }
 
-void SingletonVault::setShutdownLogOutputHandler(
-    std::function<void(std::string)> shutdownLogOutputHandlerIn) {
-  shutdownLogOutputHandler_ = std::move(shutdownLogOutputHandlerIn);
-}
-
 } // namespace folly

folly/Singleton.h

@@ -539,17 +539,9 @@ class SingletonVault {
     shutdownTimeout_ = std::chrono::milliseconds::zero();
   }
 
-  // For testing only.
-  //
-  // We want to be able to test the output messages outputted by
-  // folly::Singleton on failure to shutdown, but because of the way gtest death
-  // tests fork, we fail with an assertion failure complaining about accessing a
-  // singleton post-fork in the child process instead of the expected output log
-  void setShutdownLogOutputHandler(std::function<void(std::string)>);
-
   void addToShutdownLog(std::string message);
 
-  void fireShutdownTimer();
+  [[noreturn]] void fireShutdownTimer();
 
   void setFailOnUseAfterFork(bool failOnUseAfterFork) {
     failOnUseAfterFork_ = failOnUseAfterFork;
@@ -576,8 +568,6 @@ class SingletonVault {
 
   void startShutdownTimer();
 
-  [[noreturn]] static void defaultShutdownLogOutputHandler(std::string message);
-
   typedef std::unordered_map<
       detail::TypeDescriptor,
       detail::SingletonHolderBase*,
@@ -614,8 +604,6 @@ class SingletonVault {
   std::atomic<bool> shutdownTimerStarted_{false};
   std::chrono::milliseconds shutdownTimeout_{std::chrono::minutes{5}};
   Synchronized<std::vector<std::string>> shutdownLog_;
-  std::function<void(std::string)> shutdownLogOutputHandler_{
-      defaultShutdownLogOutputHandler};
   // We use a lock around CancellationSource to get the guarantee that all
   // cancellation callbacks that got triggered on requestCancellation() are done
   // executing by the time we start destruction.  This prevents silent callbacks

folly/test/SingletonTest.cpp

@@ -33,7 +33,6 @@ FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
 
 using namespace folly;
 using namespace std::chrono_literals;
-using namespace testing;
 
 TEST(Singleton, MissingSingleton) {
   EXPECT_DEATH(
@@ -1099,64 +1098,6 @@ TEST(Singleton, ShutdownTimerDisable) {
   vault.destroyInstancesFinal();
 }
 
-TEST(Singleton, OutputShutdownLogConstruction) {
-  // TSAN will SIGSEGV if the shutdown timer activates (it spawns a new thread,
-  // which TSAN doesn't like).
-  SKIP_IF(folly::kIsSanitizeThread);
-
-  auto finishDestructionBaton = folly::Baton<>{};
-
-  struct VaultTag {};
-  struct PrivateTag {};
-  struct Object {
-    explicit Object(folly::Baton<>& finishDestructionBaton)
-        : finishDestructionBaton_{finishDestructionBaton} {}
-
-    ~Object() { finishDestructionBaton_.wait(); }
-
-    folly::Baton<>& finishDestructionBaton_;
-  };
-  using SingletonObject = Singleton<Object, PrivateTag, VaultTag>;
-  auto typeDescriptor =
-      detail::TypeDescriptor{typeid(Object), typeid(PrivateTag)};
-
-  auto& vault = *SingletonVault::singleton<VaultTag>();
-  SingletonObject object{[&]() { return new Object{finishDestructionBaton}; }};
-  vault.registrationComplete();
-
-  vault.setShutdownTimeout(5s);
-  SingletonObject::try_get();
-
-  auto testCompletedBaton = folly::Baton<>{};
-  auto testFinishBaton = folly::Baton<>{};
-  vault.setShutdownLogOutputHandler([&](auto msg) {
-    EXPECT_TRUE(
-        msg.find("Failed to complete shutdown within 5000ms") !=
-        std::string::npos)
-        << msg;
-    EXPECT_TRUE(
-        msg.find(fmt::format("Destroying {}", typeDescriptor.name())) !=
-        std::string::npos)
-        << msg;
-    EXPECT_TRUE(
-        msg.find(fmt::format("Creating {}", typeDescriptor.name())) !=
-        std::string::npos)
-        << msg;
-
-    // we want the test to get to this point, and actually run the above
-    // expectation
-    testCompletedBaton.post();
-  });
-
-  auto waitForSignalThread = std::thread{[&] {
-    testCompletedBaton.wait();
-    finishDestructionBaton.post();
-  }};
-
-  vault.destroyInstancesFinal();
-  waitForSignalThread.join();
-}
-
 TEST(Singleton, ForkInChild) {
   struct VaultTag {};
   struct PrivateTag {};