logging: fix a crash when using logging after main() returns

Summary:
Fix a problem where `LoggerDB::get()` could crash when called after `main()`
has returned.

The `LoggerDBSingleton` object may have been destroyed already, so calling
`LoggerDBSingleton::getDB()` was not allowed.  This updates the code to just
store the singleton in a simple raw pointer, and only use the singleton helper
object for flushing log handlers.

Reviewed By: dmaone

Differential Revision: D13984692

fbshipit-source-id: a0c8550af367458ca39fefa9090e3165ad6a82bb

folly/logging/LoggerDB.cpp

@@ -69,50 +69,43 @@ FOLLY_ATTR_WEAK void initializeLoggerDB(LoggerDB& db) {
 }
 
 namespace {
-class LoggerDBSingleton {
+class LoggerDBCleanup {
  public:
-  explicit LoggerDBSingleton(LoggerDB* FOLLY_NONNULL db) : db_{db} {
-    // Call initializeLoggerDB() to apply some basic initial configuration.
-    initializeLoggerDB(*db_);
-  }
-
-  ~LoggerDBSingleton() {
-    // We intentionally leak the LoggerDB object on normal destruction.
-    // We want Logger objects to remain valid for the entire lifetime of the
-    // program, without having to worry about destruction ordering issues, or
-    // making the Logger perform reference counting on the LoggerDB.
-    //
-    // Therefore the main LoggerDB object, and all of the LogCategory objects
-    // it contains, are always intentionally leaked.
-    //
-    // However, we do call db_->cleanupHandlers() to destroy any registered
-    // LogHandler objects.  The LogHandlers can be user-defined objects and may
-    // hold resources that should be cleaned up.  This also ensures that the
-    // LogHandlers flush all outstanding messages before we exit.
+  explicit LoggerDBCleanup(LoggerDB* FOLLY_NONNULL db) : db_{db} {}
+  ~LoggerDBCleanup() {
     db_->cleanupHandlers();
-
-    // Store the released pointer in a static variable just to prevent ASAN
-    // from complaining that we are leaking data.
-    static LoggerDB* db = db_.release();
-    (void)db;
-  }
-
-  LoggerDB& getDB() const {
-    return *db_;
   }
 
  private:
-  // Store LoggerDB as a unique_ptr so it will be automatically destroyed if
-  // initializeLoggerDB() throws in the constructor.  We will explicitly
-  // release this during the normal destructor.
-  std::unique_ptr<LoggerDB> db_;
+  LoggerDB* const db_;
 };
 } // namespace
 
+LoggerDB* LoggerDB::createSingleton() {
+  // Use a unique_ptr() to clean up the LoggerDB if initializeLoggerDB() throws
+  std::unique_ptr<LoggerDB> db(new LoggerDB());
+  initializeLoggerDB(*db);
+
+  // Once initializeLoggerDB() has succeeded extract the raw pointer to return
+  // to our caller.
+  //
+  // We intentionally leak the LoggerDB singleton and all of the LogCategory
+  // objects it contains.
+  //
+  // We want Logger objects to remain valid for the entire lifetime of the
+  // program, without having to worry about destruction ordering issues, or
+  // making the Logger perform reference counting on the LoggerDB.
+  return db.release();
+}
+
 LoggerDB& LoggerDB::get() {
-  // Intentionally leaky singleton
-  static LoggerDBSingleton singleton{new LoggerDB()};
-  return singleton.getDB();
+  // Intentionally leaky LoggerDB singleton
+  static LoggerDB* const db = createSingleton();
+  // LoggerDBCleanup is responsible for calling db->cleanupHandlers() on program
+  // shutdown.  This allows log handlers to flush any buffered messages before
+  // the program exits.
+  static LoggerDBCleanup cleanup(db);
+  return *db;
 }
 
 LoggerDB::LoggerDB() {

folly/logging/LoggerDB.h

@@ -270,6 +270,7 @@ class LoggerDB {
       StringPiece categoryName,
       const std::vector<std::string>& categoryHandlerNames);
 
+  static LoggerDB* createSingleton();
   static void internalWarningImpl(
       folly::StringPiece filename,
       int lineNumber,

folly/logging/test/helpers/LogAfterMain.cpp

+/*
+ * Copyright 2017-present 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/init/Init.h>
+#include <folly/logging/test/helpers/LogOnShutdownLib.h>
+#include <folly/logging/test/helpers/helpers.h>
+#include <folly/logging/xlog.h>
+
+// Logging after main() returns is safe, but there isn't any guarantee the
+// messages will actually be visible: order of destruction is undefined, and the
+// logging handlers may have already been flushed and cleaned up by the time the
+// log messages are processed.
+LogOnDestruction d1("1");
+LogOnDestruction d2("2");
+
+int main(int argc, char* argv[]) {
+  auto init = folly::Init(&argc, &argv);
+  XLOG(INFO) << "main running";
+  use_log_on_shutdown();
+  return 0;
+}

folly/logging/test/helpers/LogAfterMainNoInit.cpp

+/*
+ * Copyright 2017-present 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/logging/test/helpers/helpers.h>
+
+LogOnDestruction d1("1");
+LogOnDestruction d2("2");
+
+int main(int, char*[]) {
+  // Never initialize folly logging.
+  return 0;
+}

folly/logging/test/helpers/LogOnShutdownLib.cpp

+/*
+ * Copyright 2019-present 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/logging/test/helpers/LogOnShutdownLib.h>
+
+#include <folly/logging/test/helpers/helpers.h>
+
+void use_log_on_shutdown() {
+  // This function doesn't do anything.
+  // It's only purpose is to make sure main() uses a symbol from this file,
+  // forcing it to be linked into the program when building statically.
+}
+
+static LogOnDestruction log_on_shutdown("log_on_shutdown");

folly/logging/test/helpers/LogOnShutdownLib.h

+/*
+ * Copyright 2019-present 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.
+ */
+#pragma once
+
+void use_log_on_shutdown();

folly/logging/test/helpers/helpers.h

+/*
+ * Copyright 2017-present 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/logging/xlog.h>
+
+class LogOnDestruction {
+ public:
+  // The constructor does not copy the message simply so we can
+  // keep it constexpr
+  constexpr LogOnDestruction(const char* msg) : msg_{msg} {}
+  ~LogOnDestruction() {
+    XLOGF(INFO, "LogOnDestruction({}) destroying", msg_);
+  }
+
+ private:
+  const char* msg_{nullptr};
+};
+
+class LogOnConstruction {
+ public:
+  template <typename... Args>
+  LogOnConstruction(folly::StringPiece fmt, Args&&... args) {
+    XLOGF(INFO, fmt, std::forward<Args>(args)...);
+  }
+};

folly/logging/test/log_after_main.py

+#!/usr/bin/env python3
+#
+# Copyright 2004-present 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.
+#
+import os
+import subprocess
+import unittest
+
+
+class LogAfterMain(unittest.TestCase):
+    def find_helper(self, name, env_var):
+        path = os.environ.get(env_var)
+        if path:
+            if not os.access(path, os.X_OK):
+                raise Exception(
+                    "path specified by $%s does not exist: %r" % (env_var, path)
+                )
+            return path
+
+        helper_subdir = os.path.join("folly", "logging", "test", "helpers")
+        buck_build_dir = os.path.join(os.getcwd(), "buck-out", "gen")
+        candidate_dirs = (
+            os.path.join(buck_build_dir, helper_subdir),
+            helper_subdir,
+            os.path.join(os.getcwd(), "helpers"),
+        )
+        for d in candidate_dirs:
+            path = os.path.join(d, name)
+            if os.access(path, os.X_OK):
+                return path
+        raise Exception("unable to find helper program %r" % (name,))
+
+    def run_helper(self, cmd):
+        return subprocess.run(
+            cmd,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            encoding="utf-8",
+            errors="surrogateescape",
+        )
+
+    def test_log_after_main(self):
+        helper = self.find_helper("log_after_main", "FOLLY_LOG_AFTER_MAIN_HELPER")
+        proc = self.run_helper([helper])
+        self.assertEqual(proc.stdout, "")
+        self.assertIn("main running", proc.stderr)
+        self.assertEqual(proc.returncode, 0, "stderr: %s" % (proc.stderr,))
+
+    def test_log_after_main_no_init(self):
+        helper = self.find_helper(
+            "log_after_main_no_init", "FOLLY_LOG_AFTER_MAIN_NO_INIT_HELPER"
+        )
+        proc = self.run_helper([helper])
+        self.assertEqual(proc.stdout, "")
+        self.assertEqual(proc.returncode, 0, "stderr: %s" % (proc.stderr,))