commit 885fcfb6c9fe
Author: Eric Rahm <erahm@mozilla.com>
Date: Thu Feb 8 16:25:07 2018 -0800
Bug 1436768 - Avoid initializing LogModuleManager more than once. r=froydnj a=RyanVM
This adds some assertions to make the intended usage of LogModuleManager::Init
more clear.
--HG--
extra : source : ec9f62910277ae252c3fb32166358419afb96b34
---
xpcom/base/Logging.cpp | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git xpcom/base/Logging.cpp xpcom/base/Logging.cpp
index 9d6a6d9ea861..eca3e97ee61e 100644
--- xpcom/base/Logging.cpp
+++ 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