Fix Crst level violation error on SendMethodDetails event

src/coreclr/src/inc/CrstTypes.def

@@ -504,12 +504,7 @@ End
 // used to remember which types have been logged (to avoid duplicate logging of the
 // same type).
 Crst EtwTypeLogHash
-    AcquiredAfter ThreadStore AllowedFiles Cer TPMethodTable
-    AcquiredBefore AvailableParamTypes ConnectionNameTable DeadlockDetection DebuggerController
-               DebuggerHeapLock DebuggerJitInfo DynamicIL ExecuteManRangeLock HandleTable IbcProfile
-               JitGenericHandleCache JumpStubCache LoaderHeap ModuleLookupTable ProfilingAPIStatus
-               ProfilerGCRefDataFreeList RWLock SingleUseLock SyncBlockCache SystemDomainDelayedUnloadList
-               ThreadIdDispenser ThreadStaticDataHashTable
+    AcquiredAfter AllowedFiles Cer SingleUseLock TPMethodTable
 End
 
 Crst Remoting

src/coreclr/src/inc/crsttypes.h

@@ -179,7 +179,7 @@ enum CrstType
 // An array mapping CrstType to level.
 int g_rgCrstLevelMap[] =
 {
-    8,          // CrstAllowedFiles
+    7,          // CrstAllowedFiles
     10,         // CrstAppDomainCache
     14,         // CrstAppDomainHandleTable
     0,          // CrstArgBasedStubCache
@@ -191,7 +191,7 @@ int g_rgCrstLevelMap[] =
     4,          // CrstAvailableParamTypes
     7,          // CrstBaseDomain
     -1,         // CrstCCompRC
-    8,          // CrstCer
+    7,          // CrstCer
     13,         // CrstClassFactInfoHash
     11,         // CrstClassInit
     -1,         // CrstClrNotification
@@ -219,7 +219,7 @@ int g_rgCrstLevelMap[] =
     0,          // CrstDynamicIL
     3,          // CrstDynamicMT
     3,          // CrstDynLinkZapItems
-    7,          // CrstEtwTypeLogHash
+    0,          // CrstEtwTypeLogHash
     18,         // CrstEventPipe
     0,          // CrstEventStore
     0,          // CrstException
@@ -321,7 +321,7 @@ int g_rgCrstLevelMap[] =
     4,          // CrstThreadStaticDataHashTable
     12,         // CrstThreadStore
     8,          // CrstTieredCompilation
-    8,          // CrstTPMethodTable
+    7,          // CrstTPMethodTable
     3,          // CrstTypeEquivalenceMap
     10,         // CrstTypeIDMap
     3,          // CrstUMEntryThunkCache

src/coreclr/src/vm/eventtrace.cpp

@@ -3462,7 +3462,7 @@ BOOL ETW::TypeSystemLog::AddTypeToGlobalCacheIfNotExists(TypeHandle th, BOOL * p
 
     BOOL fSucceeded = FALSE;
 
-    {
+   {
         CrstHolder _crst(GetHashCrst());
 
         // Check if ETW is enabled, and if not, bail here.
@@ -3487,55 +3487,85 @@ BOOL ETW::TypeSystemLog::AddTypeToGlobalCacheIfNotExists(TypeHandle th, BOOL * p
                 return fSucceeded;
             }
         }
+    }
 
-        // Step 1: go from LoaderModule to hash of types.
+    // Step 1: go from LoaderModule to hash of types.
+    Module * pLoaderModule = th.GetLoaderModule();
+    _ASSERTE(pLoaderModule != NULL);
+    LoggedTypesFromModule * pLoggedTypesFromModule = nullptr;
+    {
+        CrstHolder _crst(GetHashCrst());
+        pLoggedTypesFromModule = s_pAllLoggedTypes->allLoggedTypesHash.Lookup(pLoaderModule);
+    }
 
-        Module * pLoaderModule = th.GetLoaderModule();
-        _ASSERTE(pLoaderModule != NULL);
-        LoggedTypesFromModule * pLoggedTypesFromModule = s_pAllLoggedTypes->allLoggedTypesHash.Lookup(pLoaderModule);
+    if (pLoggedTypesFromModule == NULL)
+    {
+        pLoggedTypesFromModule = new (nothrow) LoggedTypesFromModule(pLoaderModule);
         if (pLoggedTypesFromModule == NULL)
         {
-            pLoggedTypesFromModule = new (nothrow) LoggedTypesFromModule(pLoaderModule);
-            if (pLoggedTypesFromModule == NULL)
-            {
-                // out of memory.  Bail on ETW stuff
-                *pfCreatedNew = FALSE;
-                return fSucceeded;
-            }
-
-            fSucceeded = FALSE;
-            EX_TRY
+            // out of memory.  Bail on ETW stuff
+            *pfCreatedNew = FALSE;
+            return fSucceeded;
+        }
+        {
+            CrstHolder _crst(GetHashCrst());
+            // recheck if the type has been added by another thread since we last checked above
+            LoggedTypesFromModule * recheckLoggedTypesFromModule = s_pAllLoggedTypes->allLoggedTypesHash.Lookup(pLoaderModule);
+            if (recheckLoggedTypesFromModule == NULL)
             {
-                s_pAllLoggedTypes->allLoggedTypesHash.Add(pLoggedTypesFromModule);
-                fSucceeded = TRUE;
+                EX_TRY
+                {
+                    s_pAllLoggedTypes->allLoggedTypesHash.Add(pLoggedTypesFromModule);
+                    fSucceeded = TRUE;
+                }
+                EX_CATCH
+                {
+                    fSucceeded = FALSE;
+                }
+                EX_END_CATCH(RethrowTerminalExceptions);
             }
-            EX_CATCH
+            else
             {
-                fSucceeded = FALSE;
+                delete pLoggedTypesFromModule;
+                pLoggedTypesFromModule = recheckLoggedTypesFromModule;
             }
-            EX_END_CATCH(RethrowTerminalExceptions);
+
             if (!fSucceeded)
             {
                 *pfCreatedNew = FALSE;
                 return fSucceeded;
             }
         }
+    }
 
-        // Step 2: From hash of types, see if our TypeHandle is there already
-        TypeLoggingInfo typeLoggingInfoPreexisting = pLoggedTypesFromModule->loggedTypesFromModuleHash.Lookup(th);
+    // Step 2: From hash of types, see if our TypeHandle is there already
+    TypeLoggingInfo typeLoggingInfoPreexisting;
+    {
+        CrstHolder _crst(GetHashCrst());
+        typeLoggingInfoPreexisting = pLoggedTypesFromModule->loggedTypesFromModuleHash.Lookup(th);
         if (!typeLoggingInfoPreexisting.th.IsNull())
         {
             // Type is already hashed, so it's already logged, so we don't need to
             // log it again.
             *pfCreatedNew = FALSE;
             return fSucceeded;
         }
+    }
+
+    // We haven't logged this type, so we need to continue with this function to
+    // log it below. Add it to the hash table first so any recursive calls will
+    // see that this type is already being taken care of
+    fSucceeded = FALSE;
+    TypeLoggingInfo typeLoggingInfoNew(th);
+    {
+        CrstHolder _crst(GetHashCrst());
+        // Like above, check if the type has been added from a different thread since we last looked it up.
+        if (pLoggedTypesFromModule->loggedTypesFromModuleHash.Lookup(th).th.IsNull())
+        {
+            *pfCreatedNew = FALSE;
+            return fSucceeded;
+        }
 
-        // We haven't logged this type, so we need to continue with this function to
-        // log it below. Add it to the hash table first so any recursive calls will
-        // see that this type is already being taken care of
-        fSucceeded = FALSE;
-        TypeLoggingInfo typeLoggingInfoNew(th);
         EX_TRY
         {
             pLoggedTypesFromModule->loggedTypesFromModuleHash.Add(typeLoggingInfoNew);
@@ -3555,7 +3585,6 @@ BOOL ETW::TypeSystemLog::AddTypeToGlobalCacheIfNotExists(TypeHandle th, BOOL * p
 
     *pfCreatedNew = TRUE;
     return fSucceeded;
-
 }
 
 //---------------------------------------------------------------------------------------