Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix race condition caused by updating interface map with exact target #55308

Merged
merged 1 commit into from
Jul 8, 2021
Merged

Fix race condition caused by updating interface map with exact target #55308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/coreclr/vm/methodtable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,8 +713,10 @@ PTR_MethodTable InterfaceInfo_t::GetApproxMethodTable(Module * pContainingModule

return pItfMT;
}
#endif
MethodTable * pItfMT = m_pMethodTable.GetValue();
#else
MethodTable * pItfMT = GetMethodTable();
#endif
ClassLoader::EnsureLoaded(TypeHandle(pItfMT), CLASS_LOAD_APPROXPARENTS);
return pItfMT;
}
Expand Down Expand Up @@ -9269,7 +9271,7 @@ MethodTable::ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc*
{
if (it.CurrentInterfaceMatches(this, pInterfaceType))
{
// This is the variant interface check logic, skip the
// This is the variant interface check logic, skip the exact matches as they were handled above
continue;
}

Expand Down
24 changes: 21 additions & 3 deletions src/coreclr/vm/methodtable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,36 @@ struct InterfaceInfo_t
#endif

// Method table of the interface
#ifdef FEATURE_PREJIT
#if defined(FEATURE_NGEN_RELOCS_OPTIMIZATIONS)
RelativeFixupPointer<PTR_MethodTable> m_pMethodTable;
#else
FixupPointer<PTR_MethodTable> m_pMethodTable;
#endif
#else
PTR_MethodTable m_pMethodTable;
#endif

public:
FORCEINLINE PTR_MethodTable GetMethodTable()
{
LIMITED_METHOD_CONTRACT;
#ifdef FEATURE_PREJIT
return ReadPointerMaybeNull(this, &InterfaceInfo_t::m_pMethodTable);
#else
return VolatileLoadWithoutBarrier(&m_pMethodTable);
#endif
}

#ifndef DACCESS_COMPILE
void SetMethodTable(MethodTable * pMT)
{
LIMITED_METHOD_CONTRACT;
#ifdef FEATURE_PREJIT
m_pMethodTable.SetValueMaybeNull(pMT);
#else
return VolatileStoreWithoutBarrier(&m_pMethodTable, pMT);
#endif
}

// Get approximate method table. This is used by the type loader before the type is fully loaded.
Expand All @@ -132,7 +144,11 @@ struct InterfaceInfo_t
#ifndef DACCESS_COMPILE
InterfaceInfo_t(InterfaceInfo_t &right)
{
#ifdef FEATURE_PREJIT
m_pMethodTable.SetValueMaybeNull(right.m_pMethodTable.GetValueMaybeNull());
#else
VolatileStoreWithoutBarrier(&m_pMethodTable, VolatileLoadWithoutBarrier(&right.m_pMethodTable));
#endif
}
#else // !DACCESS_COMPILE
private:
Expand Down Expand Up @@ -2222,12 +2238,14 @@ class MethodTable
}
CONTRACT_END;

bool exactMatch = m_pMap->GetMethodTable() == pMT;
MethodTable *pCurrentMethodTable = m_pMap->GetMethodTable();

bool exactMatch = pCurrentMethodTable == pMT;
if (!exactMatch)
{
if (m_pMap->GetMethodTable()->HasSameTypeDefAs(pMT) &&
if (pCurrentMethodTable->HasSameTypeDefAs(pMT) &&
pMT->HasInstantiation() &&
m_pMap->GetMethodTable()->IsSpecialMarkerTypeForGenericCasting() &&
pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() &&
pMT->GetInstantiation().ContainsAllOneType(pMTOwner))
{
exactMatch = true;
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/vm/methodtable.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1578,7 +1578,16 @@ FORCEINLINE BOOL MethodTable::ImplementsInterfaceInline(MethodTable *pInterface)

do
{
if (pInfo->GetMethodTable()->HasSameTypeDefAs(pInterface) && pInfo->GetMethodTable()->IsSpecialMarkerTypeForGenericCasting())
MethodTable *pInterfaceInMap = pInfo->GetMethodTable();
if (pInterfaceInMap == pInterface)
{
// Since there is no locking on updating the interface with an exact match
// It is possible to reach here for a match which would ideally have been handled above
// GetMethodTable uses a VolatileLoadWithoutBarrier to prevent compiler optimizations
// from interfering with this check
return TRUE;
}
if (pInterfaceInMap->HasSameTypeDefAs(pInterface) && pInterfaceInMap->IsSpecialMarkerTypeForGenericCasting())
{
// Extensible RCW's need to be handled specially because they can have interfaces
// in their map that are added at runtime. These interfaces will have a start offset
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/siginfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,11 +1417,13 @@ TypeHandle SigPointer::GetTypeHandleThrowing(

Instantiation genericLoadInst(thisinst, ntypars);

#ifndef FEATURE_PREJIT // PREJIT doesn't support the volatile update semantics in the interface map that this requires
if (pMTInterfaceMapOwner != NULL && genericLoadInst.ContainsAllOneType(pMTInterfaceMapOwner))
{
thRet = ClassLoader::LoadTypeDefThrowing(pGenericTypeModule, tkGenericType, ClassLoader::ThrowIfNotFound, ClassLoader::PermitUninstDefOrRef, 0, level);
}
else
#endif // FEATURE_PREJIT
{
// Group together the current signature type context and substitution chain, which
// we may later use to instantiate constraints of type arguments that turn out to be
Expand Down