Lock all access to logging data, in case it's reinitialized

base/logging.cc

Index: base/logging.cc
diff --git a/base/logging.cc b/base/logging.cc
index f90a3695f2bd55be2554ece0617edc5ec12c9796..946fe4002a6fc0fb468c419e44d2d8d3c8c41dc3 100644
--- a/base/logging.cc
+++ b/base/logging.cc
@@ -60,7 +60,6 @@ const char* const log_severity_names[LOG_NUM_SEVERITIES] = {
   "INFO", "WARNING", "ERROR", "ERROR_REPORT", "FATAL" };
 
 int min_log_level = 0;
-LogLockingState lock_log_file = LOCK_LOG_FILE;
 
 // The default set here for logging_destination will only be used if
 // InitLogging is not called.  On Windows, use a file next to the exe;
@@ -109,19 +108,6 @@ LogReportHandlerFunction log_report_handler = NULL;
 // A log message handler that gets notified of every log message we process.
 LogMessageHandlerFunction log_message_handler = NULL;
 
-// The lock is used if log file locking is false. It helps us avoid problems
-// with multiple threads writing to the log file at the same time.  Use
-// LockImpl directly instead of using Lock, because Lock makes logging calls.
-static LockImpl* log_lock = NULL;
-
-// When we don't use a lock, we are using a global mutex. We need to do this
-// because LockFileEx is not thread safe.
-#if defined(OS_WIN)
-MutexHandle log_mutex = NULL;
-#elif defined(OS_POSIX)
-pthread_mutex_t log_mutex = PTHREAD_MUTEX_INITIALIZER;
-#endif
-
 // Helper functions to wrap platform differences.
 
 int32 CurrentProcessId() {
@@ -178,6 +164,105 @@ void DeleteFilePath(const PathString& log_name) {
 #endif
 }
 
+// This class acts as a wrapper for locking the logging files.
+// LoggingLock::Init() should be called from the main thread before any logging
+// is done. Then whenever logging, be sure to have a local LoggingLock
+// instance on the stack. This will ensure that the lock is unlocked upon
+// exiting the frame.
+// LoggingLocks can not be nested.
+class LoggingLock {
+ public:
+  LoggingLock() {
+    LockLogging();
+  }
+
+  ~LoggingLock() {
+    UnlockLogging();
+  }
+
+  static void Init(LogLockingState lock_log) {
+    if (initialized)
+      return;
+    lock_log_file = lock_log;
+    if (lock_log_file == LOCK_LOG_FILE) {
+#if defined(OS_WIN)
+      if (!log_mutex) {
+        // \ is not a legal character in mutex names so we replace \ with /
+        std::wstring safe_name(*log_file_name);
+        std::replace(safe_name.begin(), safe_name.end(), '\\', '/');
+        std::wstring t(L"Global\\");
+        t.append(safe_name);
+        log_mutex = ::CreateMutex(NULL, FALSE, t.c_str());
+      }
+#endif
+    } else {
+      log_lock = new LockImpl();
+    }
+    initialized = true;
+  }
+
+ private:
+  static void LockLogging() {
+    if (lock_log_file == LOCK_LOG_FILE) {
+#if defined(OS_WIN)
+      ::WaitForSingleObject(log_mutex, INFINITE);
+      // WaitForSingleObject could have returned WAIT_ABANDONED. We don't
+      // abort the process here. UI tests might be crashy sometimes,
+      // and aborting the test binary only makes the problem worse.
+      // We also don't use LOG macros because that might lead to an infinite
+      // loop. For more info see http://crbug.com/18028.
+#elif defined(OS_POSIX)
+      pthread_mutex_lock(&log_mutex);
+#endif
+    } else {
+      // use the lock
+      log_lock->Lock();
+    }
+  }
+
+  static void UnlockLogging() {
+    if (lock_log_file == LOCK_LOG_FILE) {
+#if defined(OS_WIN)
+      ReleaseMutex(log_mutex);
+#elif defined(OS_POSIX)
+      pthread_mutex_unlock(&log_mutex);
+#endif
+    } else {
+      log_lock->Unlock();
+    }
+  }
+
+  // The lock is used if log file locking is false. It helps us avoid problems
+  // with multiple threads writing to the log file at the same time.  Use
+  // LockImpl directly instead of using Lock, because Lock makes logging calls.
+  static LockImpl* log_lock;
+
+  // When we don't use a lock, we are using a global mutex. We need to do this
+  // because LockFileEx is not thread safe.
+#if defined(OS_WIN)
+  static MutexHandle log_mutex;
+#elif defined(OS_POSIX)
+  static pthread_mutex_t log_mutex;
+#endif
+
+  static bool initialized;
+  static LogLockingState lock_log_file;
+};
+
+// static
+bool LoggingLock::initialized = false;
+// static
+LockImpl* LoggingLock::log_lock = NULL;
+// static
+LogLockingState LoggingLock::lock_log_file = LOCK_LOG_FILE;
+
+#if defined(OS_WIN)
+// static
+MutexHandle LoggingLock::log_mutex = NULL;
+#elif defined(OS_POSIX)
+pthread_mutex_t LoggingLock::log_mutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
 // Called by logging functions to ensure that debug_file is initialized
 // and can be used for writing. Returns false if the file could not be
 // initialized. debug_file will be NULL in this case.
@@ -231,21 +316,6 @@ bool InitializeLogFileHandle() {
   return true;
 }
 
-void InitLogMutex() {
-#if defined(OS_WIN)
-  if (!log_mutex) {
-    // \ is not a legal character in mutex names so we replace \ with /
-    std::wstring safe_name(*log_file_name);
-    std::replace(safe_name.begin(), safe_name.end(), '\\', '/');
-    std::wstring t(L"Global\\");
-    t.append(safe_name);
-    log_mutex = ::CreateMutex(NULL, FALSE, t.c_str());
-  }
-#elif defined(OS_POSIX)
-  // statically initialized
-#endif
-}
-
 void BaseInitLoggingImpl(const PathChar* new_log_file,
                          LoggingDestination logging_dest,
                          LogLockingState lock_log,
@@ -253,6 +323,10 @@ void BaseInitLoggingImpl(const PathChar* new_log_file,
   g_enable_dcheck =
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableDCHECK);
 
+  LoggingLock::Init(lock_log);
+
+  LoggingLock logging_lock;
+
   if (log_file) {
     // calling InitLogging twice or after some log call has already opened the
     // default log file will re-initialize to the new options
@@ -260,7 +334,6 @@ void BaseInitLoggingImpl(const PathChar* new_log_file,
     log_file = NULL;
   }
 
-  lock_log_file = lock_log;
   logging_destination = logging_dest;
 
   // ignore file options if logging is disabled or only to system
@@ -274,13 +347,8 @@ void BaseInitLoggingImpl(const PathChar* new_log_file,
   if (delete_old == DELETE_OLD_LOG_FILE)
     DeleteFilePath(*log_file_name);
 
-  if (lock_log_file == LOCK_LOG_FILE) {
-    InitLogMutex();
-  } else if (!log_lock) {
-    log_lock = new LockImpl();
-  }
-
   InitializeLogFileHandle();
+
 }
 
 void SetMinLogLevel(int level) {
@@ -517,61 +585,31 @@ LogMessage::~LogMessage() {
     fflush(stderr);
   }
 
+  // We can have multiple threads and/or processes, so try to prevent them
+  // from clobbering each other's writes.
+  // If the client app did not call InitLogging, and the lock has not
+  // been created do it now. We do this on demand, but if two threads try
+  // to do this at the same time, there will be a race condition to create
+  // the lock. This is why InitLogging should be called from the main
+  // thread at the beginning of execution.
+  LoggingLock::Init(LOCK_LOG_FILE);
   // write to log file
   if (logging_destination != LOG_NONE &&
-      logging_destination != LOG_ONLY_TO_SYSTEM_DEBUG_LOG &&
-      InitializeLogFileHandle()) {
-    // We can have multiple threads and/or processes, so try to prevent them
-    // from clobbering each other's writes.
-    if (lock_log_file == LOCK_LOG_FILE) {
-      // Ensure that the mutex is initialized in case the client app did not
-      // call InitLogging. This is not thread safe. See below.
-      InitLogMutex();
-
+      logging_destination != LOG_ONLY_TO_SYSTEM_DEBUG_LOG) {
+    LoggingLock logging_lock;
+    if (InitializeLogFileHandle()) {
 #if defined(OS_WIN)
-      ::WaitForSingleObject(log_mutex, INFINITE);
-      // WaitForSingleObject could have returned WAIT_ABANDONED. We don't
-      // abort the process here. UI tests might be crashy sometimes,
-      // and aborting the test binary only makes the problem worse.
-      // We also don't use LOG macros because that might lead to an infinite
-      // loop. For more info see http://crbug.com/18028.
-#elif defined(OS_POSIX)
-      pthread_mutex_lock(&log_mutex);
-#endif
-    } else {
-      // use the lock
-      if (!log_lock) {
-        // The client app did not call InitLogging, and so the lock has not
-        // been created. We do this on demand, but if two threads try to do
-        // this at the same time, there will be a race condition to create
-        // the lock. This is why InitLogging should be called from the main
-        // thread at the beginning of execution.
-        log_lock = new LockImpl();
-      }
-      log_lock->Lock();
-    }
-
-#if defined(OS_WIN)
-    SetFilePointer(log_file, 0, 0, SEEK_END);
-    DWORD num_written;
-    WriteFile(log_file,
-              static_cast<const void*>(str_newline.c_str()),
-              static_cast<DWORD>(str_newline.length()),
-              &num_written,
-              NULL);
+      SetFilePointer(log_file, 0, 0, SEEK_END);
+      DWORD num_written;
+      WriteFile(log_file,
+                static_cast<const void*>(str_newline.c_str()),
+                static_cast<DWORD>(str_newline.length()),
+                &num_written,
+                NULL);
 #else
-    fprintf(log_file, "%s", str_newline.c_str());
-    fflush(log_file);
+      fprintf(log_file, "%s", str_newline.c_str());
+      fflush(log_file);
 #endif
-
-    if (lock_log_file == LOCK_LOG_FILE) {
-#if defined(OS_WIN)
-      ReleaseMutex(log_mutex);
-#elif defined(OS_POSIX)
-      pthread_mutex_unlock(&log_mutex);
-#endif
-    } else {
-      log_lock->Unlock();
     }
   }
 
@@ -695,6 +733,8 @@ ErrnoLogMessage::~ErrnoLogMessage() {
 #endif  // OS_WIN
 
 void CloseLogFile() {
+  LoggingLock logging_lock;
+
   if (!log_file)
     return;