You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thread A calls GetStubForInteropMethod and gets back pCode = X, then thread B does the same and gets pCode = Y. Thread A's SetCodeEntryPoint(X) publishes X as GetNativeCode(). Thread B then hits the assert because Y != X.
// NOTE: there is a race in updating this MethodDesc. We depend on all// threads getting back the same DynamicMethodDesc for a particular// PInvokeMethodDesc, in that case, the locking around the actual JIT// operation will prevent the code from being jitted more than once.// By the time we get here, all threads get the same address of code// back from the JIT operation and they all just fill in the same value// here.
This invariant depends on all threads resolving to the sameDynamicMethodDesc (ILStub) via the ILStubCache. However, the cache lookup and insert in ILStubCache::GetStubMethodDesc (ilstubcache.cpp#L514-L586) is guarded by SF_IsSharedStub:
if (SF_IsSharedStub(dwStubFlags))
{
CrstHolder ch(&m_crst);
const ILStubCacheEntry* phe = m_hashMap.LookupPtr(pHashBlob);
if (phe)
{
pMD = phe->m_pMethodDesc;
}
}
For forward P/Invokes (non-CALLI, non-vararg), IsSharedStubScenario returns false (dllimport.cpp#L232-L235):
if (SF_IsForwardPInvokeStub(dwStubFlags) && !SF_IsCALLIStub(dwStubFlags) && !SF_IsVarArgStub(dwStubFlags))
{
returnfalse;
}
So SF_IsSharedStub is never set for ordinary P/Invokes, which means:
The hash-based cache lookup is skipped entirely
CreateNewMethodDesc is called unconditionally — each racing thread gets a distinctMethodDesc*
The subsequent JitILStub produces different PCODE values (same code, different addresses)
SetCodeEntryPoint in DoPrestub sees a mismatch and asserts
Root Cause
PR #117901 removed shared IL stubs for P/Invokes by making IsSharedStubScenario return false for forward P/Invokes. This was previously the mechanism that ensured all threads resolved to the same DynamicMethodDesc. With it gone, concurrent threads each create their own stub and JIT it independently, producing distinct PCODE values — invalidating the comment at dllimport.cpp#L5689.
Should P/Invoke ILStub creation re-enable cache lookup/insert (i.e., revert the IsSharedStubScenario change for the cache path, even if the stubs are no longer "shared" in the old sense)?
Or should DoPrestub handle pCode != GetNativeCode() for P/Invokes by discarding the duplicate and using the already-published value?
Or should synchronization be added in PInvoke::GetStubForILStub so only one thread creates the stub per MethodDesc?
Repro Notes
Add ClrSleepEx(100, FALSE) after GetStubForInteropMethod at prestub.cpp#L2284 to make repro more consistent
Run System.Globalization.Tests with dotnet build src\libraries\System.Runtime\tests\System.Globalization.Tests\System.Globalization.Tests.csproj /p:TestReadyToRun=true /t:test
Note, this only reproduces with ReadyToRun enabled. When running with ReadyToRun, HasPrecode is false, so GetNativeCode() returns the code pointed to by the method. However, without ReadyToRun, HasPrecode returns true for the PInvoke that causes failures. Then GetNativeCode() short circuits to return NULL, and the Assert doesn't fire. I'm still not sure where or why CoreCLR sets HasPrecode while loading the method for a fixup doesn't, but this seems to be why the failure doesn't repro without R2R. Applying the following diff (with the delay) demonstrates that without R2R, we are still jitting the stub multiple times and have multiple entrypoints that we try to set the entry point to - though it does look like the final set is atomic. We probably don't need a backport for the fix.
Details
--- a/src/coreclr/vm/method.cpp+++ b/src/coreclr/vm/method.cpp@@ -3254,7 +3254,16 @@ void MethodDesc::SetCodeEntryPoint(PCODE entryPoint)
// Use this path if there already exists a Precode, OR if RequiresStableEntryPoint is set.
//
// RequiresStableEntryPoint currently requires that the entrypoint must be a Precode
- GetOrCreatePrecode()->SetTargetInterlocked(entryPoint);+ Precode* pPrecode = GetOrCreatePrecode();+ BOOL wonSetTargetRace = pPrecode->SetTargetInterlocked(entryPoint);+#ifdef _DEBUG+ if (!wonSetTargetRace && IsPInvoke())+ {+ // For P/Invoke, losing the precode publication race should still mean that all racing+ // threads computed the same target entry point.+ _ASSERTE(pPrecode->GetTarget() == entryPoint);+ }+#endif // _DEBUG
return;
}
Summary
Multiple threads can race in P/Invoke ILStub creation and produce different
PCODEvalues for the sameMethodDesc, violating the assert inDoPrestub.Failing Assert
prestub.cpp#L2340:
_ASSERTE(pCode == (PCODE)NULL || GetNativeCode() == (PCODE)NULL || pCode == GetNativeCode());Thread A calls
GetStubForInteropMethodand gets backpCode= X, then thread B does the same and getspCode= Y. Thread A'sSetCodeEntryPoint(X)publishes X asGetNativeCode(). Thread B then hits the assert becauseY != X.Why different stubs are produced
The existing comment at dllimport.cpp#L5689-L5699 claims this can't happen:
This invariant depends on all threads resolving to the same
DynamicMethodDesc(ILStub) via theILStubCache. However, the cache lookup and insert inILStubCache::GetStubMethodDesc(ilstubcache.cpp#L514-L586) is guarded bySF_IsSharedStub:For forward P/Invokes (non-CALLI, non-vararg),
IsSharedStubScenarioreturnsfalse(dllimport.cpp#L232-L235):So
SF_IsSharedStubis never set for ordinary P/Invokes, which means:CreateNewMethodDescis called unconditionally — each racing thread gets a distinctMethodDesc*JitILStubproduces differentPCODEvalues (same code, different addresses)SetCodeEntryPointinDoPrestubsees a mismatch and assertsRoot Cause
PR #117901 removed shared IL stubs for P/Invokes by making
IsSharedStubScenarioreturnfalsefor forward P/Invokes. This was previously the mechanism that ensured all threads resolved to the sameDynamicMethodDesc. With it gone, concurrent threads each create their own stub and JIT it independently, producing distinctPCODEvalues — invalidating the comment at dllimport.cpp#L5689.Relevant Call Path
Questions
IsSharedStubScenariochange for the cache path, even if the stubs are no longer "shared" in the old sense)?DoPrestubhandlepCode != GetNativeCode()for P/Invokes by discarding the duplicate and using the already-published value?PInvoke::GetStubForILStubso only one thread creates the stub perMethodDesc?Repro Notes
ClrSleepEx(100, FALSE)afterGetStubForInteropMethodat prestub.cpp#L2284 to make repro more consistentdotnet build src\libraries\System.Runtime\tests\System.Globalization.Tests\System.Globalization.Tests.csproj /p:TestReadyToRun=true /t:testNote, this only reproduces with ReadyToRun enabled. When running with ReadyToRun, HasPrecode is false, so GetNativeCode() returns the code pointed to by the method. However, without ReadyToRun, HasPrecode returns true for the PInvoke that causes failures. Then GetNativeCode() short circuits to return NULL, and the Assert doesn't fire. I'm still not sure where or why CoreCLR sets HasPrecode while loading the method for a fixup doesn't, but this seems to be why the failure doesn't repro without R2R. Applying the following diff (with the delay) demonstrates that without R2R, we are still jitting the stub multiple times and have multiple entrypoints that we try to set the entry point to - though it does look like the final set is atomic. We probably don't need a backport for the fix.
Details
Summary generated with AI