Reduce funceval abort (#108256)

Visual Studio reported that they were seeing unnecessary func-eval aborts. This was due to a lock ordering issue between CrstReadyToRunEntryPointToMethodDescMap and the coop mode transition. Flipping the ordering should fix the issue for this particular lock though it doesn't prevent any other lock from blocking func-evals. This should reduce, but not eliminate, the number of cases where func-eval abort is required.

Co-authored-by: Noah Falk <[email protected]>
Co-authored-by: Tom McDonald <[email protected]>

Author: 3 people

src/coreclr/vm/readytoruninfo.cpp

@@ -351,6 +351,12 @@ void ReadyToRunInfo::SetMethodDescForEntryPointInNativeImage(PCODE entryPoint, M
     }
     CONTRACTL_END;
 
+    // We are entering coop mode here so that we don't do it later inside LookupMap while we are already holding the Crst.
+    // Doing it in the other order can block the debugger from running func-evals. For example thread A would acquire the Crst,
+    // then block at the coop transition inside LookupMap waiting for the debugger to resume from a break state. The debugger then
+    // requests thread B to run a funceval, the funceval tries to load some R2R method calling in here, then it blocks because
+    // thread A is holding the Crst.
+    GCX_COOP();
     CrstHolder ch(&m_Crst);
 
     if ((TADDR)m_entryPointToMethodDescMap.LookupValue(PCODEToPINSTR(entryPoint), (LPVOID)PCODEToPINSTR(entryPoint)) == (TADDR)INVALIDENTRY)
@@ -697,7 +703,7 @@ ReadyToRunInfo::ReadyToRunInfo(Module * pModule, LoaderAllocator* pLoaderAllocat
     m_pHeader(pHeader),
     m_pNativeImage(pModule != NULL ? pNativeImage: NULL), // m_pNativeImage is only set for composite image components, not the composite R2R info itself
     m_readyToRunCodeDisabled(FALSE),
-    m_Crst(CrstReadyToRunEntryPointToMethodDescMap),
+    m_Crst(CrstReadyToRunEntryPointToMethodDescMap, CRST_UNSAFE_COOPGC),
     m_pPersistentInlineTrackingMap(NULL),
     m_pNextR2RForUnrelatedCode(NULL)
 {