Fix logging initialization race (#616) (#634)
authorAnubhav Srivastava <SrivastavaAnubhav@users.noreply.github.com>
Mon, 9 Dec 2019 10:47:40 +0000 (02:47 -0800)
committerJan Vorlicek <janvorli@microsoft.com>
Mon, 9 Dec 2019 10:47:40 +0000 (11:47 +0100)
* Make logging initialization thread-safe. Fix broken defines for InterlockedCompareExchangePointer and InterlockedExchangePointer.

* Add InitLogging back to log.h. Revert change to the defines in utilcode.h. Move LogFileMutex initialization to InitializeLogging.

src/coreclr/src/utilcode/log.cpp

index 6f209cf11bc526391b3c9ff97ccee86520b29a44..e9bc9257253a7c357cbd8342c6bb3636b7fc043d 100644 (file)
 #define LOG_ENABLE                      0x0040
 
 
-static DWORD    LogFlags                    = 0;
-static CQuickWSTR     szLogFileName;
-static HANDLE   LogFileHandle               = INVALID_HANDLE_VALUE;
-static MUTEX_COOKIE   LogFileMutex                = 0;
-static DWORD    LogFacilityMask             = LF_ALL;
-static DWORD    LogFacilityMask2            = 0;
-static DWORD    LogVMLevel                  = LL_INFO100;
+static          DWORD        LogFlags                    = 0;
+static          CQuickWSTR   szLogFileName;
+static          HANDLE       LogFileHandle               = INVALID_HANDLE_VALUE;
+static volatile MUTEX_COOKIE LogFileMutex                = 0;
+static          DWORD        LogFacilityMask             = LF_ALL;
+static          DWORD        LogFacilityMask2            = 0;
+static          DWORD        LogVMLevel                  = LL_INFO100;
         // <TODO>@todo FIX should probably only display warnings and above by default</TODO>
 
 
@@ -96,15 +96,6 @@ VOID InitLogging()
             FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN |  ((LogFlags & LOG_ENABLE_FLUSH_FILE) ? FILE_FLAG_WRITE_THROUGH : 0),
             NULL);
 
-        if(0 == LogFileMutex)
-        {
-            LogFileMutex = ClrCreateMutex(
-                NULL,
-                FALSE,
-                NULL);
-            _ASSERTE(LogFileMutex != 0);
-        }
-
             // Some other logging may be going on, try again with another file name
         if (LogFileHandle == INVALID_HANDLE_VALUE && wcslen(szLogFileName.Ptr()) + 3 <= szLogFileName.Size())
         {
@@ -187,16 +178,28 @@ VOID LeaveLogLock()
     }
 }
 
-static bool bLoggingInitialized = false;
+static volatile bool bLoggingInitialized = false;
 VOID InitializeLogging()
 {
     STATIC_CONTRACT_NOTHROW;
 
     if (bLoggingInitialized)
         return;
-    bLoggingInitialized = true;
 
-    InitLogging();      // You can call this in the debugger to fetch new settings
+    MUTEX_COOKIE mutexCookie = ClrCreateMutex(NULL, FALSE, NULL);
+    _ASSERTE(mutexCookie != 0);
+    if (InterlockedCompareExchangeT(&LogFileMutex, mutexCookie, 0) != 0)
+    {
+        ClrCloseMutex(mutexCookie);
+    }
+
+    EnterLogLock();
+    if (!bLoggingInitialized)
+    {
+        InitLogging();      // You can call this in the debugger to fetch new settings
+        bLoggingInitialized = true;
+    }
+    LeaveLogLock();
 }
 
 VOID FlushLogging() {