From 87736606f3e1f415c1ffcadf51c5d2cc77421815 Mon Sep 17 00:00:00 2001
From: Eric Rahm <erahm@mozilla.com>
Date: Thu, 8 Feb 2018 16:25:07 -0800
Subject: [PATCH] Bug 1436768 - Avoid initializing LogModuleManager more than
 once. r=froydnj

This adds some assertions to make the intended usage of LogModuleManager::Init
more clear.

--HG--
extra : rebase_source : c61e1736cedfbeaa96951ba40cdc954bbc0094d5
---
 xpcom/base/Logging.cpp | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/xpcom/base/Logging.cpp b/xpcom/base/Logging.cpp
index 9d6a6d9ea861..eca3e97ee61e 100644
--- a/xpcom/base/Logging.cpp
+++ b/xpcom/base/Logging.cpp
@@ -16,6 +16,7 @@
 #include "mozilla/Atomics.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/UniquePtrExtensions.h"
+#include "MainThreadUtils.h"
 #include "nsClassHashtable.h"
 #include "nsDebug.h"
 #include "NSPRLogModulesParser.h"
@@ -179,6 +180,7 @@ public:
     , mAddTimestamp(false)
     , mIsSync(false)
     , mRotate(0)
+    , mInitialized(false)
   {
   }
 
@@ -190,9 +192,17 @@ public:
 
   /**
    * Loads config from env vars if present.
+   *
+   * Notes:
+   *
+   * 1) This function is only intended to be called once per session.
+   * 2) None of the functions used in Init should rely on logging.
    */
   void Init()
   {
+    MOZ_DIAGNOSTIC_ASSERT(!mInitialized);
+    mInitialized = true;
+
     bool shouldAppend = false;
     bool addTimestamp = false;
     bool isSync = false;
@@ -213,8 +223,10 @@ public:
       }
     }
 
+    // Need to capture `this` since `sLogModuleManager` is not set until after
+    // initialization is complete.
     NSPRLogModulesParser(modules,
-        [&shouldAppend, &addTimestamp, &isSync, &rotate]
+        [this, &shouldAppend, &addTimestamp, &isSync, &rotate]
             (const char* aName, LogLevel aLevel, int32_t aValue) mutable {
           if (strcmp(aName, "append") == 0) {
             shouldAppend = true;
@@ -225,7 +237,7 @@ public:
           } else if (strcmp(aName, "rotate") == 0) {
             rotate = (aValue << 20) / kRotateFilesNumber;
           } else {
-            LogModule::Get(aName)->SetLevel(aLevel);
+            this->CreateOrGetModule(aName)->SetLevel(aLevel);
           }
     });
 
@@ -513,6 +525,7 @@ private:
   Atomic<bool, Relaxed> mAddTimestamp;
   Atomic<bool, Relaxed> mIsSync;
   int32_t mRotate;
+  bool mInitialized;
 };
 
 StaticAutoPtr<LogModuleManager> sLogModuleManager;
@@ -557,6 +570,8 @@ LogModule::Init()
 {
   // NB: This method is not threadsafe; it is expected to be called very early
   //     in startup prior to any other threads being run.
+  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
+
   if (sLogModuleManager) {
     // Already initialized.
     return;
@@ -565,8 +580,12 @@ LogModule::Init()
   // NB: We intentionally do not register for ClearOnShutdown as that happens
   //     before all logging is complete. And, yes, that means we leak, but
   //     we're doing that intentionally.
-  sLogModuleManager = new LogModuleManager();
-  sLogModuleManager->Init();
+
+  // Don't assign the pointer until after Init is called. This should help us
+  // detect if any of the functions called by Init somehow rely on logging.
+  auto mgr = new LogModuleManager();
+  mgr->Init();
+  sLogModuleManager = mgr;
 }
 
 void