Skip to content

Fix concurrent CCW vtable and IID publication #135650

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

Merged
AaronRobinsonMSFT merged 5 commits into from
Oct 16, 2026
Merged

Fix concurrent CCW vtable and IID publication #135650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f7a869
Initial plan
Copilot Oct 12, 2026
ce1fd2e
Fix monotonic CCW vtable and IID publication
Copilot Oct 12, 2026
77de2f7
Use full-width interlocked OR for CCW flags
Copilot Oct 13, 2026
d19447b
Remove non-reproducing concurrent CCW stress test
Copilot Oct 14, 2026
fa37c5a
Simplify CCW flags and descriptor bookkeeping
AaronRobinsonMSFT Oct 15, 2026
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
85 changes: 33 additions & 52 deletions src/coreclr/vm/comcallablewrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2723,7 +2723,7 @@ LONG ComCallWrapperCache::Release()

//--------------------------------------------------------------------------
// void ComMethodTable::Cleanup()
// free the stubs and the vtable
// Release the cached dispatch and type information.
//--------------------------------------------------------------------------
void ComMethodTable::Cleanup()
{
Expand All @@ -2735,37 +2735,12 @@ void ComMethodTable::Cleanup()
}
CONTRACTL_END;

unsigned cbExtraSlots = GetNumExtraSlots(GetInterfaceType());
unsigned cbSlots = m_cbSlots;

SLOT* pComVtable = (SLOT *)(this + 1);

// If we have created and laid out the method desc then we need to delete them.
if (IsLayoutComplete())
{
#ifdef PROFILING_SUPPORTED
// We used to issue the COMClassicVTableDestroyed callback from here.
// However, that causes an AV. At this point the MethodTable is gone
// (as the AppDomain containing it has been unloaded), but the ComMethodTable
// still points to it. The code here used to wrap a TypeHandle around the
// MethodTable pointer, cast to a ClassID, and then call COMClassicVTableDestroyed.
// But the act of casting to a TypeHandle invokes debug-code to verify the
// MethodTable, which causes an AV.
//
// For now, we're not issuing the COMClassicVTableDestroyed callback anymore.
// <REVISIT_TODO>Reexamine the profiling API around
// CCWs and move the callback elsewhere and / or rethink the current
// set of CCW callbacks to mirror reality more accurately.</REVISIT_TODO>
#endif // PROFILING_SUPPORTED
}

if (m_pDispatchInfo)
delete m_pDispatchInfo;
if (m_pITypeInfo && !IsAtProcessExit())
SafeRelease(m_pITypeInfo);

// The m_pMDescr and the current instance is allocated from the related LoaderAllocator
// so no cleanup is needed here.
// The method descriptors, entry thunks, and this instance belong to the LoaderAllocator.
}

namespace
Expand Down Expand Up @@ -2813,8 +2788,6 @@ void ComMethodTable::LayOutClassMethodTable()
IDispatchVtable* pDispVtable;
SLOT *pComVtable;
unsigned cbPrevSlots = 0;
unsigned cbAlloc = 0;
BYTE* pMDMemoryPtr = nullptr;
AllocMemTracker amTracker;
LoaderAllocator* pLoaderAllocator = m_pMT->GetLoaderAllocator();
ComCallMethodDesc* pMethodDescMemory = NULL;
Expand Down Expand Up @@ -2914,18 +2887,12 @@ void ComMethodTable::LayOutClassMethodTable()
// Allocate method desc's for the rest of the slots.
//
unsigned cbMethodDescs = sizeof(ComCallMethodDesc) * (m_cbSlots - cbParentComMTSlots);
cbAlloc = cbMethodDescs;
if (cbAlloc > 0)
if (cbMethodDescs > 0)
{
pMDMemoryPtr = (BYTE*)amTracker.Track(m_pMT->GetLoaderAllocator()->GetLowFrequencyHeap()->AllocMem(S_SIZE_T(cbAlloc + sizeof(UINT_PTR))));
pMethodDescMemory = (ComCallMethodDesc*)amTracker.Track(pLoaderAllocator->GetLowFrequencyHeap()->AllocMem(S_SIZE_T(cbMethodDescs)));

// initialize the method desc memory to zero
FillMemory(pMDMemoryPtr, cbAlloc + sizeof(UINT_PTR), 0x0);

*(UINT_PTR *)(pMDMemoryPtr) = cbMethodDescs; // fill in the size of the method desc's

// move past the size
pMethodDescMemory = (ComCallMethodDesc*)(pMDMemoryPtr + sizeof(UINT_PTR));
FillMemory(pMethodDescMemory, cbMethodDescs, 0x0);
}

_ASSERTE(0 == (((DWORD_PTR)pMethodDescMemory) & (sizeof(void*)-1)));
Expand Down Expand Up @@ -3099,17 +3066,12 @@ void ComMethodTable::LayOutClassMethodTable()
// IDispatch vtable follows the header
CopyMemory(this + 1, pDispVtable, cbTempVtable.Value());

// Set the layout complete flag and release the lock.
m_Flags |= enum_LayoutComplete;
amTracker.SuppressRelease();

// We've successfully laid out the class method table so we need to suppress the release of the
// memory for the ComCallMethodDescs and store it inside the ComMethodTable so we can
// release it when we clean up the ComMethodTable.
m_pMDescr = (BYTE*)pMDMemoryPtr;
// Publish only after the descriptors and allocation ownership are committed.
SetFlags(enum_LayoutComplete);
}

amTracker.SuppressRelease();

LOG((LF_INTEROP, LL_INFO1000, "LayOutClassMethodTable: %s, parent: %s, this: %p [DONE]\n", m_pMT->GetDebugClassName(), pParentClass ? pParentClass->GetDebugClassName() : 0, this));
}

Expand Down Expand Up @@ -3290,7 +3252,7 @@ BOOL ComMethodTable::LayOutInterfaceMethodTable(MethodTable* pClsMT)
_ASSERTE(numVtableSlots == cbEmittedSlots);

// Set the layout complete flag and release the lock.
m_Flags |= enum_LayoutComplete;
SetFlags(enum_LayoutComplete);
}

#ifdef PROFILING_SUPPORTED
Expand Down Expand Up @@ -3337,7 +3299,7 @@ void ComMethodTable::LayOutBasicMethodTable()

//
// Set up the IUnknown and IDispatch methods. Each thread will write exactly the same values to the
// slots so we let it run concurrently and execute a memory barrier by doing InterlockOr at the end.
// slots so we let it run concurrently and publish them with an atomic flag update at the end.
//

// IDispatch vtable follows the header
Expand Down Expand Up @@ -3373,11 +3335,33 @@ void ComMethodTable::LayOutBasicMethodTable()
//
// Set the layout complete flag.
//
InterlockedOr((LONG*)&m_Flags, enum_LayoutComplete);
SetFlags(enum_LayoutComplete);

LOG((LF_INTEROP, LL_INFO1000, "LayOutClassMethodTable: %s, this: %p [DONE]\n", m_pMT->GetDebugClassName(), this));
}

void ComMethodTable::GenerateIID()
{
CONTRACTL
{
THROWS;
GC_TRIGGERS;
MODE_ANY;
}
CONTRACTL_END;

// Metadata generation can throw and trigger GC, so do it outside the commit lock.
IID iid;
GenerateClassItfGuid(TypeHandle(m_pMT), &iid);

CrstHolder ch(&g_CreateWrapperTemplateCrst);
if (!(VolatileLoad(&m_Flags) & enum_GuidGenerated))
{
m_IID = iid;
SetFlags(enum_GuidGenerated);
}
}

//--------------------------------------------------------------------------
// Retrieves the DispatchInfo associated with the COM method table. If
// the DispatchInfo has not been initialized yet then it is initialized.
Expand Down Expand Up @@ -3964,7 +3948,6 @@ ComMethodTable* ComCallWrapperTemplate::CreateComMethodTableForClass(MethodTable
pComMT->m_ptReserved = (SLOT)(size_t)0xDEADC0FF; // reserved
pComMT->m_pMT = pClassMT; // pointer to the class method table
pComMT->m_cbRefCount = 0;
pComMT->m_pMDescr = NULL;
pComMT->m_pITypeInfo = NULL;
pComMT->m_pDispatchInfo = NULL;
pComMT->m_cbSlots = cbTotalSlots.Value(); // number of slots not counting IDisp methods.
Expand Down Expand Up @@ -4038,7 +4021,6 @@ ComMethodTable* ComCallWrapperTemplate::CreateComMethodTableForInterface(MethodT
pComMT->m_pMT = pInterfaceMT; // pointer to the interface's method table
pComMT->m_cbSlots = cbComSlots; // number of slots not counting IUnk
pComMT->m_cbRefCount = 0;
pComMT->m_pMDescr = NULL;
pComMT->m_pITypeInfo = NULL;
pComMT->m_pDispatchInfo = NULL;

Expand Down Expand Up @@ -4097,7 +4079,6 @@ ComMethodTable* ComCallWrapperTemplate::CreateComMethodTableForBasic(MethodTable
pComMT->m_pMT = pMT;
pComMT->m_cbSlots = 0; // number of slots not counting IUnk
pComMT->m_cbRefCount = 0;
pComMT->m_pMDescr = NULL;
pComMT->m_pITypeInfo = NULL;
pComMT->m_pDispatchInfo = NULL;

Expand Down
60 changes: 22 additions & 38 deletions src/coreclr/vm/comcallablewrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ struct ComMethodTable
BOOL IsLayoutComplete()
Comment thread
AaronRobinsonMSFT marked this conversation as resolved.
Resolved
{
LIMITED_METHOD_CONTRACT;
return (m_Flags & enum_LayoutComplete) != 0;
return (VolatileLoad(&m_Flags) & enum_LayoutComplete) != 0;
}

BOOL IsComVisible()
Expand Down Expand Up @@ -558,38 +558,6 @@ struct ComMethodTable
return (ComCallMethodDesc *)pCMD;
}

BOOL OwnedbyThisMT(unsigned slotIndex)
{
CONTRACTL
{
WRAPPER(THROWS);
WRAPPER(GC_TRIGGERS);
MODE_ANY;
}
CONTRACTL_END;

if (!IsIClassXOrBasicItf())
return TRUE;

if (m_pMDescr != NULL)
{
// These are the methods from the default interfaces such as IUnknown.
unsigned cbExtraSlots = GetNumExtraSlots(GetInterfaceType());

// Refer to ComMethodTable::LayOutClassMethodTable().
ULONG cbSize = *(ULONG *)m_pMDescr;
ULONG cbNewSlots = cbSize / sizeof(ComCallMethodDesc);
_ASSERTE( (cbSize % sizeof(ComCallMethodDesc)) == 0);

// m_cbSlots is the total number of methods in addition to the ones from the
// default interfaces. cbNewSlots is the total number of methods introduced
// by this class (== m_cbSlots - <slots from parent MT>).
return slotIndex >= (cbExtraSlots + m_cbSlots - cbNewSlots);
}

return FALSE;
}

ComMethodTable *GetParentClassComMT();

static inline PTR_ComMethodTable ComMethodTableFromIP(PTR_IUnknown pUnk)
Expand Down Expand Up @@ -639,10 +607,9 @@ struct ComMethodTable
CONTRACTL_END;

// Generate the IClassX IID if it hasn't been generated yet.
if (!(m_Flags & enum_GuidGenerated))
if (!(VolatileLoad(&m_Flags) & enum_GuidGenerated))
{
GenerateClassItfGuid(TypeHandle(m_pMT), &m_IID);
m_Flags |= enum_GuidGenerated;
GenerateIID();
}

return m_IID;
Expand All @@ -657,12 +624,29 @@ struct ComMethodTable
}

private:
#ifndef DACCESS_COMPILE
void GenerateIID();

void SetFlags(size_t flags)
{
LIMITED_METHOD_CONTRACT;

static_assert(offsetof(ComMethodTable, m_Flags) % alignof(size_t) == 0);

// Preserve other flags while publishing the payload initialized before this update.
Comment thread
AaronRobinsonMSFT marked this conversation as resolved.
Resolved
#ifdef HOST_64BIT
InterlockedOr64(reinterpret_cast<LONG64*>(&m_Flags), static_cast<LONG64>(flags));
#else
InterlockedOr(reinterpret_cast<LONG*>(&m_Flags), static_cast<LONG>(flags));
#endif // HOST_64BIT
}
#endif // DACCESS_COMPILE

SLOT m_ptReserved; //= (SLOT) 0xDEADC0FF; reserved
PTR_MethodTable m_pMT; // pointer to the VMs method table
ULONG m_cbSlots; // number of slots in the interface (excluding IUnk/IDisp)
LONG m_cbRefCount; // ref-count the vtable as it is being shared
size_t m_Flags; // make sure this is initialized to zero
LPVOID m_pMDescr; // pointer to methoddescr.s owned by this MT
ITypeInfo* m_pITypeInfo; // cached pointer to ITypeInfo
DispatchInfo* m_pDispatchInfo; // The dispatch info used to expose IDispatch to COM.
IID m_IID; // The IID of the interface.
Expand All @@ -671,7 +655,7 @@ struct ComMethodTable
// SLOT m_slots[]; // vtable entries (m_cbSlots of them, plus the 3 or 7 from IUnk/IDisp)
// For interface COM method tables, an inline ComCallMethodDesc m_comCallMethodDesc[] array (m_cbSlots entries)
// may follow the slots. For class-interface layouts, the ComCallMethodDesc[] block is allocated separately and
// referenced via m_pMDescr. Basic COM method tables may have no ComCallMethodDesc descriptors at all.
// referenced by the entry thunks. Basic COM method tables may have no ComCallMethodDesc descriptors at all.

friend struct ::cdac_data<ComMethodTable>;
};
Expand Down
Loading