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

Review URL: http://codereview.chromium.org/3195014

TBR=davemoore@chromium.org
Problems with IE tests for ChromeFrame and heapcheck.

Review URL: http://codereview.chromium.org/3229012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@58079 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 100 insertions, 148 deletions

diff --git a/base/logging.cc b/base/logging.cc
index b7c313e..f90a369 100644
--- a/base/logging.cc
+++ b/base/logging.cc
@@ -60,6 +60,7 @@ 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;
@@ -108,6 +109,19 @@ 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() {
@@ -164,125 +178,6 @@ void DeleteFilePath(const PathString& log_name) {
 #endif
 }
 
-PathString* GetDefaultLogFile() {
-#if defined(OS_WIN)
-  // On Windows we use the same path as the exe.
-  wchar_t module_name[MAX_PATH];
-  GetModuleFileName(NULL, module_name, MAX_PATH);
-  PathString *new_log_file = new std::wstring(module_name);
-  std::wstring::size_type last_backslash =
-    new_log_file->rfind('\\', new_log_file->size());
-  if (last_backslash != std::wstring::npos)
-    new_log_file->erase(last_backslash + 1);
-  *new_log_file += L"debug.log";
-  return new_log_file;
-#elif defined(OS_POSIX)
-  // On other platforms we just use the current directory.
-  return new std::string("debug.log");
-#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, PathString* new_log_file) {
-    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 /
-        if (!new_log_file)
-          new_log_file = GetDefaultLogFile();
-        std::wstring safe_name(*new_log_file);
-        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.
@@ -293,7 +188,20 @@ bool InitializeLogFileHandle() {
   if (!log_file_name) {
     // Nobody has called InitLogging to specify a debug log file, so here we
     // initialize the log file name to a default.
-    log_file_name = GetDefaultLogFile();
+#if defined(OS_WIN)
+    // On Windows we use the same path as the exe.
+    wchar_t module_name[MAX_PATH];
+    GetModuleFileName(NULL, module_name, MAX_PATH);
+    log_file_name = new std::wstring(module_name);
+    std::wstring::size_type last_backslash =
+        log_file_name->rfind('\\', log_file_name->size());
+    if (last_backslash != std::wstring::npos)
+      log_file_name->erase(last_backslash + 1);
+    *log_file_name += L"debug.log";
+#elif defined(OS_POSIX)
+    // On other platforms we just use the current directory.
+    log_file_name = new std::string("debug.log");
+#endif
   }
 
   if (logging_destination == LOG_ONLY_TO_FILE ||
@@ -323,17 +231,27 @@ 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,
                          OldFileDeletionState delete_old) {
   g_enable_dcheck =
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableDCHECK);
-  PathString* new_log_file_name = new PathString();
-  *new_log_file_name = new_log_file;
-  LoggingLock::Init(lock_log, new_log_file_name);
-
-  LoggingLock logging_lock;
 
   if (log_file) {
     // calling InitLogging twice or after some log call has already opened the
@@ -342,6 +260,7 @@ 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
@@ -355,8 +274,13 @@ void BaseInitLoggingImpl(const PathChar* new_log_file,
   if (delete_old == DELETE_OLD_LOG_FILE)
     DeleteFilePath(*log_file_name);
 
-  InitializeLogFileHandle();
+  if (lock_log_file == LOCK_LOG_FILE) {
+    InitLogMutex();
+  } else if (!log_lock) {
+    log_lock = new LockImpl();
+  }
 
+  InitializeLogFileHandle();
 }
 
 void SetMinLogLevel(int level) {
@@ -593,31 +517,61 @@ 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, NULL);
   // write to log file
   if (logging_destination != LOG_NONE &&
-      logging_destination != LOG_ONLY_TO_SYSTEM_DEBUG_LOG) {
-    LoggingLock logging_lock;
-    if (InitializeLogFileHandle()) {
+      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();
+
 #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);
+      ::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);
 #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();
     }
   }
 
@@ -741,8 +695,6 @@ ErrnoLogMessage::~ErrnoLogMessage() {
 #endif  // OS_WIN
 
 void CloseLogFile() {
-  LoggingLock logging_lock;
-
   if (!log_file)
     return;