Avoid glog when warning about Singleton double-registration
authorYedidya Feldblum <yfeldblum@fb.com>
Mon, 2 May 2016 09:26:07 +0000 (02:26 -0700)
committerFacebook Github Bot 8 <facebook-github-bot-8-bot@fb.com>
Mon, 2 May 2016 09:35:28 +0000 (02:35 -0700)
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

folly/Singleton-inl.h
folly/Singleton.cpp
folly/test/SingletonDoubleRegistration.cpp [new file with mode: 0644]
folly/test/SingletonTest.cpp

index 83ef3926a6dfe1c9e26d05251d94ef26e45f3750..0071ddf5fe752e1fe9edc9366756d97bc4b444d3 100644 (file)
@@ -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);
index 8e9cd99b0763138bd8eb7c61819da5c293a61703..0468679772946a1071de998d5642459e379264b0 100644 (file)
 #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 (file)
index 0000000..207472c
--- /dev/null
@@ -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;
+}
index b36b47091123d28b265683fc470faf7c506cfac0..6603df2140a47447318a67af4b339dbe7d3ea923 100644 (file)
 #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"));
+}