Correctly lock around iterating the AvailableParamTypes hashtable when adding fields/classes via ApplyMetadata (#118608)

This does not eliminate all possible race conditions here, if a type is in progress loading when the metadata is updated, then there is still a race, but this should narrow the possible races in this space. Additionally, this should address the debug asserts which have been hit in this space.

Fixes #115318

Author: davidwrighton

src/coreclr/vm/class.cpp

@@ -391,6 +391,8 @@ HRESULT EEClass::AddField(MethodTable* pMT, mdFieldDef fieldDef, FieldDesc** ppN
             LOG((LF_ENC, LL_INFO100, "EEClass::AddField Checking: %s mod:%p\n", pMod->GetDebugName(), pMod));
 
             EETypeHashTable* paramTypes = pMod->GetAvailableParamTypes();
+            CrstHolder ch(pMod->GetClassLoader()->GetAvailableTypesLock());
+
             EETypeHashTable::Iterator it(paramTypes);
             EETypeHashEntry* pEntry;
             while (paramTypes->FindNext(&it, &pEntry))
@@ -587,6 +589,8 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc**
             LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Checking: %s mod:%p\n", pMod->GetDebugName(), pMod));
 
             EETypeHashTable* paramTypes = pMod->GetAvailableParamTypes();
+            CrstHolder ch(pMod->GetClassLoader()->GetAvailableTypesLock());
+
             EETypeHashTable::Iterator it(paramTypes);
             EETypeHashEntry* pEntry;
             while (paramTypes->FindNext(&it, &pEntry))

src/coreclr/vm/clsload.cpp

@@ -1735,7 +1735,7 @@ VOID ClassLoader::Init(AllocMemTracker *pamTracker)
     // This lock is taken within the classloader whenever we have to insert a new param. type into the table.
     m_AvailableTypesLock.Init(
                               CrstAvailableParamTypes,
-                              CRST_DEBUGGER_THREAD);
+                              CrstFlags(CRST_DEBUGGER_THREAD | CRST_GC_NOTRIGGER_WHEN_TAKEN | CRST_UNSAFE_ANYMODE));
 
 #ifdef _DEBUG
     CorTypeInfo::CheckConsistency();

src/coreclr/vm/clsload.hpp

@@ -533,6 +533,13 @@ class ClassLoader
 
 
 public:
+
+    CrstBase *GetAvailableTypesLock()
+    {
+        LIMITED_METHOD_CONTRACT;
+        return &m_AvailableTypesLock;
+    }
+
     //#LoaderModule
     // LoaderModule determines in which module an item gets placed.
     // For everything except parameterized types and methods the choice is easy.

src/coreclr/vm/excep.cpp

@@ -10824,7 +10824,7 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr)
     CONTRACTL
     {
         THROWS;
-        DISABLED(GC_NOTRIGGER);  // Must sanitize first pass handling to enable this
+        GC_NOTRIGGER;
         MODE_ANY;
     }
     CONTRACTL_END;
@@ -10837,7 +10837,7 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr)
     // !
     // ! COMPlusThrowHR(hr, kGetErrorInfo)
 
-    RealCOMPlusThrowHR(hr, (IErrorInfo*)NULL);
+    EX_THROW(EEMessageException, (hr));
 }
 
 VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, tagGetErrorInfo)
@@ -10883,7 +10883,7 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, UINT resID, LPCWSTR wszArg
     CONTRACTL
     {
         THROWS;
-        DISABLED(GC_NOTRIGGER);  // Must sanitize first pass handling to enable this
+        GC_NOTRIGGER;
         MODE_ANY;
     }
     CONTRACTL_END;