Bug 1407810 - Enable DDLogging from the beginning if MOZ_LOG contains DDLogger - r?jwwang draft
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 13 Oct 2017 17:29:41 +1100
changeset 680730 9d278b7b6abdd1a8bf8a9426225c4fc10af0fef5
parent 680729 ab6c2e04e8cf22c34401b28cf0ccc5fe561fc630
child 680731 167315a67c3cc0af3edda5cf9aa0aa1fa5baf7fc
push id84600
push usergsquelart@mozilla.com
push dateMon, 16 Oct 2017 07:01:56 +0000
reviewersjwwang
bugs1407810
milestone58.0a1
Bug 1407810 - Enable DDLogging from the beginning if MOZ_LOG contains DDLogger - r?jwwang This allows logging from the command line without using a webextension. MozReview-Commit-ID: 8IztpVf68CC
dom/media/doctor/DecoderDoctorLogger.cpp
dom/media/doctor/DecoderDoctorLogger.h
--- a/dom/media/doctor/DecoderDoctorLogger.cpp
+++ b/dom/media/doctor/DecoderDoctorLogger.cpp
@@ -10,17 +10,17 @@
 #include "DDMediaLogs.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/SystemGroup.h"
 #include "mozilla/Unused.h"
 
 namespace mozilla {
 
 /* static */ Atomic<DecoderDoctorLogger::LogState, ReleaseAcquire>
-  DecoderDoctorLogger::sLogState{ DecoderDoctorLogger::scDisabled };
+  DecoderDoctorLogger::sLogState{ DecoderDoctorLogger::scUninitialized };
 
 /* static */ const char* DecoderDoctorLogger::sShutdownReason = nullptr;
 
 static DDMediaLogs* sMediaLogs;
 
 // First DDLogShutdowner sets sLogState to scShutdown, to prevent further
 // logging.
 struct DDLogShutdowner
@@ -79,24 +79,61 @@ DecoderDoctorLogger::PanicInternal(const
       // trying to write or read log messages, thereby causing a UAF.
     }
     // If someone else changed the state, we'll just loop around, and either
     // shutdown already happened elsewhere, or we'll try to shutdown again.
   }
 }
 
 /* static */ bool
+DecoderDoctorLogger::CheckDDLoggingEnabled()
+{
+  for (;;) {
+    LogState state = static_cast<LogState>(sLogState);
+    switch (state) {
+      case scUninitialized:
+        // If the env-var's MOZ_LOG contains one of the DDLoggers, enable
+        // DDLogging now.
+        if (MOZ_LOG_TEST(sDecoderDoctorLoggerLog, LogLevel::Error) ||
+            MOZ_LOG_TEST(sDecoderDoctorLoggerEndLog, LogLevel::Error)) {
+          return EnsureLogIsEnabled();
+        }
+        // Otherwise let's start with disabled logging.
+        // (It may later change if a media devtools panel opens.)
+        if (sLogState.compareExchange(scUninitialized, scDisabled)) {
+          // Successfully set to disabled, we're done.
+          return false;
+        }
+        // The state was not uninitialized anymore, let's look again
+        break;
+      case scDisabled:
+        return false;
+      case scEnabled:
+        return true;
+      case scEnabling:
+        // Someone else is currently enabling logging, actively wait by just
+        // looping, until the state changes.
+        break;
+      case scShutdown:
+        // Shutdown is non-recoverable, we cannot enable logging again.
+        return false;
+    }
+  }
+}
+
+/* static */ bool
 DecoderDoctorLogger::EnsureLogIsEnabled()
 {
   for (;;) {
     LogState state = static_cast<LogState>(sLogState);
     switch (state) {
+      case scUninitialized:
       case scDisabled:
-        // Currently disabled, try to be the one to enable.
-        if (sLogState.compareExchange(scDisabled, scEnabling)) {
+        // Currently uninitialized or disabled, try to be the one to enable.
+        if (sLogState.compareExchange(state, scEnabling)) {
           // We are the one to enable logging, state won't change (except for
           // possible shutdown.)
           // Create DDMediaLogs singleton, which will process the message queue.
           DDMediaLogs::ConstructionResult mediaLogsConstruction =
             DDMediaLogs::New();
           if (NS_FAILED(mediaLogsConstruction.mRv)) {
             PanicInternal("Failed to enable logging", /* aDontBlock */ true);
             return false;
--- a/dom/media/doctor/DecoderDoctorLogger.h
+++ b/dom/media/doctor/DecoderDoctorLogger.h
@@ -36,17 +36,27 @@ namespace mozilla {
 // take too much memory.
 class DecoderDoctorLogger
 {
 public:
   // Is logging currently enabled? This is tested anyway in all public `Log...`
   // functions, but it may be used to prevent logging-only work in clients.
   static inline bool IsDDLoggingEnabled()
   {
-    return MOZ_UNLIKELY(static_cast<LogState>(sLogState) == scEnabled);
+    LogState state = sLogState;
+    // In most real-world cases, DDLogging will be disabled.
+    if (state == scDisabled) {
+      return false;
+    }
+    // Next most-likely case: Enabled.
+    if (state == scEnabled) {
+      return true;
+    }
+    // In other states, perform a thorough check.
+    return CheckDDLoggingEnabled();
   }
 
   // Shutdown logging. This will prevent more messages to be queued, but the
   // already-queued messages may still get processed.
   static void ShutdownLogging() { sLogState = scShutdown; }
 
   // Something went horribly wrong, stop all logging and log processing.
   static void Panic(const char* aReason)
@@ -314,16 +324,19 @@ public:
   // This call will trigger a processing run (to ensure the most recent data
   // will be returned), and the returned promise will be resolved with all
   // relevant log messages and object lifetimes in a JSON string.
   // The first call will enable logging, until shutdown.
   static RefPtr<LogMessagesPromise> RetrieveMessages(
     const dom::HTMLMediaElement* aMediaElement);
 
 private:
+  // Check if logging is enabled, including initializing it if needed.
+  static bool CheckDDLoggingEnabled();
+
   // If logging is not enabled yet, initiate it, return true.
   // If logging has been shutdown, don't start it, return false.
   // Otherwise return true.
   static bool EnsureLogIsEnabled();
 
   // Note that this call may block while the state is scEnabling;
   // set aDontBlock to true to avoid blocking, most importantly when the
   // caller is the one doing the enabling, this would cause an endless loop.
@@ -331,25 +344,28 @@ private:
 
   static void Log(const char* aSubjectTypeName,
                   const void* aSubjectPointer,
                   DDLogCategory aCategory,
                   const char* aLabel,
                   DDLogValue&& aValue);
 
   using LogState = int;
+  // Not yet initialized, need to check if MOZ_LOG contains DDLogger to first
+  // enable/disable.
+  static constexpr LogState scUninitialized = 0;
   // Currently disabled, may be enabled on request.
-  static constexpr LogState scDisabled = 0;
+  static constexpr LogState scDisabled = 1;
   // Currently enabled (logging happens), may be shutdown.
-  static constexpr LogState scEnabled = 1;
+  static constexpr LogState scEnabled = 2;
   // Still disabled, but one thread is working on enabling it, nobody else
   // should interfere during this time.
-  static constexpr LogState scEnabling = 2;
+  static constexpr LogState scEnabling = 3;
   // Shutdown, cannot be re-enabled.
-  static constexpr LogState scShutdown = 3;
+  static constexpr LogState scShutdown = 4;
   // Current state.
   // "ReleaseAcquire" because when changing to scEnabled, the just-created
   // sMediaLogs must be accessible to consumers that see scEnabled.
   static Atomic<LogState, ReleaseAcquire> sLogState;
 
   // If non-null, reason for an abnormal shutdown.
   static const char* sShutdownReason;
 };