From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Mon, 2 May 2016 09:26:07 +0000 (-0700)
Subject: Avoid glog when warning about Singleton double-registration
X-Git-Tag: 2016.07.26~297
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=7dd60842ca7b3ca266db5dfcf31e99b3adbfbe17;p=folly.git

Avoid glog when warning about Singleton double-registration

Summary:
[Folly] Avoid glog when warning about `Singleton` double-registration.

Because registration happens at static initialization time, long before main, and possibly long before glog is initialized.

This makes a difference because, in some cases of double-registration, we can get a SIGSEGV with no message, because we are attempting to `LOG(FATAL)` when glog is uninitialized. What we would much rather have is a SIGABRT with a message.

Reviewed By: meyering

Differential Revision: D3245047

fb-gh-sync-id: 4c5dd9d25025f197d7c490ffbb429af5ccb82182
fbshipit-source-id: 4c5dd9d25025f197d7c490ffbb429af5ccb82182
---

diff --git a/folly/Singleton-inl.h b/folly/Singleton-inl.h
index 83ef3926..0071ddf5 100644
--- a/folly/Singleton-inl.h
+++ b/folly/Singleton-inl.h
@@ -29,6 +29,9 @@ SingletonHolder<T>& SingletonHolder<T>::singleton() {
   return *entry;
 }
 
+[[noreturn]] void singletonWarnDoubleRegistrationAndAbort(
+    const TypeDescriptor& type);
+
 template <typename T>
 void SingletonHolder<T>::registerSingleton(CreateFunc c, TeardownFunc t) {
   std::lock_guard<std::mutex> entry_lock(mutex_);
@@ -51,9 +54,7 @@ void SingletonHolder<T>::registerSingleton(CreateFunc c, TeardownFunc t) {
      * Singleton<int> b([] { return new int(4); });
      *
      */
-    LOG(FATAL) << "Double registration of singletons of the same "
-               << "underlying type; check for multiple definitions "
-               << "of type folly::Singleton<" + type_.name() + ">";
+    singletonWarnDoubleRegistrationAndAbort(type_);
   }
 
   create_ = std::move(c);
diff --git a/folly/Singleton.cpp b/folly/Singleton.cpp
index 8e9cd99b..04686797 100644
--- a/folly/Singleton.cpp
+++ b/folly/Singleton.cpp
@@ -17,8 +17,12 @@
 #include <folly/Singleton.h>
 
 #include <atomic>
+#include <cstdio>
+#include <cstdlib>
+#include <sstream>
 #include <string>
 
+#include <folly/FileUtil.h>
 #include <folly/ScopeGuard.h>
 
 namespace folly {
@@ -27,6 +31,17 @@ namespace detail {
 
 constexpr std::chrono::seconds SingletonHolderBase::kDestroyWaitTime;
 
+[[noreturn]] void singletonWarnDoubleRegistrationAndAbort(
+    const TypeDescriptor& type) {
+  // Not using LOG(FATAL) or std::cerr because they may not be initialized yet.
+  std::ostringstream o;
+  o << "Double registration of singletons of the same "
+    << "underlying type; check for multiple definitions "
+    << "of type folly::Singleton<" << type.name() << ">" << std::endl;
+  auto s = o.str();
+  writeFull(STDERR_FILENO, s.data(), s.size());
+  std::abort();
+}
 }
 
 namespace {
diff --git a/folly/test/SingletonDoubleRegistration.cpp b/folly/test/SingletonDoubleRegistration.cpp
new file mode 100644
index 00000000..207472c6
--- /dev/null
+++ b/folly/test/SingletonDoubleRegistration.cpp
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2016 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/Singleton.h>
+
+folly::Singleton<int> foo;
+folly::Singleton<int> bar;
+
+int main(int, char**) {
+  return 0;
+}
diff --git a/folly/test/SingletonTest.cpp b/folly/test/SingletonTest.cpp
index b36b4709..6603df21 100644
--- a/folly/test/SingletonTest.cpp
+++ b/folly/test/SingletonTest.cpp
@@ -17,10 +17,13 @@
 #include <thread>
 
 #include <folly/Singleton.h>
+#include <folly/Subprocess.h>
+#include <folly/experimental/io/FsUtil.h>
 #include <folly/io/async/EventBase.h>
 #include <folly/test/SingletonTestStructs.h>
 
 #include <glog/logging.h>
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <boost/thread/barrier.hpp>
 
@@ -580,3 +583,20 @@ TEST(Singleton, MockTest) {
   // If serial_count value is the same, then singleton was not replaced.
   EXPECT_NE(serial_count_first, serial_count_mock);
 }
+
+TEST(Singleton, DoubleRegistrationLogging) {
+  const auto basename = "singleton_double_registration";
+  const auto sub = fs::executable_path().remove_filename() / basename;
+  auto p = Subprocess(
+      std::vector<std::string>{sub.string()},
+      Subprocess::Options()
+          .stdin(Subprocess::CLOSE)
+          .stdout(Subprocess::CLOSE)
+          .pipeStderr()
+          .closeOtherFds());
+  auto err = p.communicate("").second;
+  auto res = p.wait();
+  EXPECT_EQ(ProcessReturnCode::KILLED, res.state());
+  EXPECT_EQ(SIGABRT, res.killSignal());
+  EXPECT_THAT(err, testing::StartsWith("Double registration of singletons"));
+}