Skip to content

Commit

Permalink
Reduce funceval abort (dotnet#108256)
Browse files Browse the repository at this point in the history
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 <noahfalk@microsoft.com>
Co-authored-by: Tom McDonald <tommcdon@microsoft.com>
  • Loading branch information
3 people authored Sep 27, 2024
1 parent 41e6052 commit 3fed4b3
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/coreclr/vm/readytoruninfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down

0 comments on commit 3fed4b3

Please sign in to comment.