From 3c6bc847b2e18c93885168e214b55ea6efbc149d Mon Sep 17 00:00:00 2001 From: Noah Falk Date: Tue, 24 Sep 2024 17:12:59 -0700 Subject: [PATCH] Reduce funceval abort 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. --- src/coreclr/vm/readytoruninfo.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/readytoruninfo.cpp b/src/coreclr/vm/readytoruninfo.cpp index eed2bf26e94b03..34cd10559aa1ba 100644 --- a/src/coreclr/vm/readytoruninfo.cpp +++ b/src/coreclr/vm/readytoruninfo.cpp @@ -383,6 +383,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) @@ -729,7 +735,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) {