From d42765723606eef37c7e647e41e87538048bd3eb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 9 Mar 2026 22:44:05 +0100 Subject: [PATCH 01/13] Delete GC handles in COOP mode --- src/coreclr/binder/customassemblybinder.cpp | 2 ++ src/coreclr/debug/ee/debugger.cpp | 3 +++ src/coreclr/debug/ee/debugger.h | 1 + src/coreclr/vm/appdomain.cpp | 1 + src/coreclr/vm/clrex.cpp | 2 ++ src/coreclr/vm/comcallablewrapper.cpp | 1 + src/coreclr/vm/comconnectionpoints.h | 1 + src/coreclr/vm/dllimportcallback.cpp | 1 + src/coreclr/vm/dynamicmethod.cpp | 1 + src/coreclr/vm/eedbginterfaceimpl.cpp | 2 ++ src/coreclr/vm/encee.cpp | 17 ++++++++++------- src/coreclr/vm/exinfo.cpp | 1 + src/coreclr/vm/exinfo.h | 1 + src/coreclr/vm/gchandleutilities.h | 2 +- .../vm/interoplibinterface_comwrappers.cpp | 1 + src/coreclr/vm/jitinterface.h | 1 + src/coreclr/vm/syncblk.cpp | 1 + src/coreclr/vm/threads.cpp | 1 + 18 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/coreclr/binder/customassemblybinder.cpp b/src/coreclr/binder/customassemblybinder.cpp index 950e462d512ae5..c19267f7dddfaa 100644 --- a/src/coreclr/binder/customassemblybinder.cpp +++ b/src/coreclr/binder/customassemblybinder.cpp @@ -253,6 +253,8 @@ void CustomAssemblyBinder::ReleaseLoadContext() // This method is called to release the weak and strong handles on the managed AssemblyLoadContext // once the Unloading event has been fired OBJECTHANDLE handle = reinterpret_cast(GetAssemblyLoadContext()); + + GCX_COOP(); DestroyLongWeakHandle(handle); handle = reinterpret_cast(m_ptrManagedStrongAssemblyLoadContext); DestroyHandle(handle); diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 575392b318425d..1285520e62af25 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -10984,6 +10984,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) OBJECTHANDLE objectHandle = pEvent->DisposeHandle.vmObjectHandle.GetRawPtr(); CorDebugHandleType handleType = pEvent->DisposeHandle.handleType; + GCX_COOP(); switch (handleType) { case HANDLE_STRONG: @@ -12304,6 +12305,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF } else { + GCX_COOP(); DestroyLongWeakHandle(objHandle); } } @@ -12313,6 +12315,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF { m_pForceCatchHandlerFoundEventsTable->Remove(objHandle); } + GCX_COOP(); DestroyLongWeakHandle(objHandle); } return S_OK; diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index f1ed7e1ea70960..fd7be4ff3a7d45 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -535,6 +535,7 @@ class EMPTY_BASES_DECL ForceCatchHandlerFoundSHashTraits : public DefaultSHashTr } static void OnRemovePerEntryCleanupAction(const OBJECTHANDLE &e) { + GCX_COOP(); DestroyLongWeakHandle(e); } }; diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 88d6b1642b0f24..be594538d52af1 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -136,6 +136,7 @@ PinnedHeapHandleBucket::~PinnedHeapHandleBucket() if (m_hndHandleArray) { + GCX_COOP(); DestroyStrongHandle(m_hndHandleArray); m_hndHandleArray = NULL; } diff --git a/src/coreclr/vm/clrex.cpp b/src/coreclr/vm/clrex.cpp index 2953e0a75b5f08..6f45897b7fe7b7 100644 --- a/src/coreclr/vm/clrex.cpp +++ b/src/coreclr/vm/clrex.cpp @@ -50,6 +50,8 @@ CLRException::~CLRException() STRESS_LOG1(LF_EH, LL_INFO100, "CLRException::~CLRException destroying throwable: obj = %x\n", GetThrowableHandle()); // clear the handle first, so if we SO on destroying it, we don't have a dangling reference SetThrowableHandle(NULL); + + GCX_COOP(); DestroyHandle(throwableHandle); } } diff --git a/src/coreclr/vm/comcallablewrapper.cpp b/src/coreclr/vm/comcallablewrapper.cpp index 1b0c6eea5a4e45..d9555016582542 100644 --- a/src/coreclr/vm/comcallablewrapper.cpp +++ b/src/coreclr/vm/comcallablewrapper.cpp @@ -2014,6 +2014,7 @@ void ComCallWrapper::ClearHandle() OBJECTHANDLE pThis = m_ppThis; if (InterlockedCompareExchangeT(&m_ppThis, NULL, pThis) == pThis) { + GCX_COOP(); DestroyRefcountedHandle(pThis); } } diff --git a/src/coreclr/vm/comconnectionpoints.h b/src/coreclr/vm/comconnectionpoints.h index f4b90a16d36c4b..fc777f89430b11 100644 --- a/src/coreclr/vm/comconnectionpoints.h +++ b/src/coreclr/vm/comconnectionpoints.h @@ -50,6 +50,7 @@ struct ConnectionCookie ~ConnectionCookie() { WRAPPER_NO_CONTRACT; + GCX_COOP(); DestroyHandle(m_hndEventProvObj); } diff --git a/src/coreclr/vm/dllimportcallback.cpp b/src/coreclr/vm/dllimportcallback.cpp index 8525434710dbfd..ffc4b15737e1eb 100644 --- a/src/coreclr/vm/dllimportcallback.cpp +++ b/src/coreclr/vm/dllimportcallback.cpp @@ -319,6 +319,7 @@ void UMEntryThunkData::Terminate() if (pObjectHandle != NULL) { + GCX_COOP(); DestroyLongWeakHandle(pObjectHandle); } diff --git a/src/coreclr/vm/dynamicmethod.cpp b/src/coreclr/vm/dynamicmethod.cpp index 5939702e667534..4191543c8b6c9e 100644 --- a/src/coreclr/vm/dynamicmethod.cpp +++ b/src/coreclr/vm/dynamicmethod.cpp @@ -1103,6 +1103,7 @@ void LCGMethodResolver::DestroyResolver() if (m_managedResolver) { + GCX_COOP(); ::DestroyLongWeakHandle(m_managedResolver); m_managedResolver = NULL; } diff --git a/src/coreclr/vm/eedbginterfaceimpl.cpp b/src/coreclr/vm/eedbginterfaceimpl.cpp index bd2a782b6daf67..a39598769860ca 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.cpp +++ b/src/coreclr/vm/eedbginterfaceimpl.cpp @@ -211,6 +211,7 @@ void EEDbgInterfaceImpl::DbgDestroyHandle(OBJECTHANDLE oh, CONTRACTL { NOTHROW; + MODE_ANY; GC_NOTRIGGER; } CONTRACTL_END; @@ -218,6 +219,7 @@ void EEDbgInterfaceImpl::DbgDestroyHandle(OBJECTHANDLE oh, LOG((LF_CORDB, LL_INFO1000, "EEI::GHFO: Destroyed given handle 0x%x," "fStrong: 0x%x!\n", oh, fStrongNewRef)); + GCX_COOP(); if (fStrongNewRef) { DestroyStrongHandle(oh); diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index 025df8ac2aa77f..72c012dedef4d7 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -1474,15 +1474,18 @@ void EnCSyncBlockInfo::Cleanup() CONTRACTL_END; // Walk our linked list of all the fields that were added EnCAddedField *pEntry = m_pList; - while (pEntry) { - // Clean up the handle we created in EnCAddedField::Allocate - DestroyDependentHandle(*(OBJECTHANDLE*)&pEntry->m_FieldData); + GCX_COOP(); + while (pEntry) + { + // Clean up the handle we created in EnCAddedField::Allocate + DestroyDependentHandle(*(OBJECTHANDLE*)&pEntry->m_FieldData); - // Delete this list entry and move onto the next - EnCAddedField *next = pEntry->m_pNext; - delete pEntry; - pEntry = next; + // Delete this list entry and move onto the next + EnCAddedField *next = pEntry->m_pNext; + delete pEntry; + pEntry = next; + } } // Finally, delete the sync block info itself diff --git a/src/coreclr/vm/exinfo.cpp b/src/coreclr/vm/exinfo.cpp index a38cf79104560c..cbde40416bfa51 100644 --- a/src/coreclr/vm/exinfo.cpp +++ b/src/coreclr/vm/exinfo.cpp @@ -76,6 +76,7 @@ void ExInfo::ReleaseResources() { if (!CLRException::IsPreallocatedExceptionHandle(m_hThrowable)) { + GCX_COOP(); DestroyHandle(m_hThrowable); } m_hThrowable = NULL; diff --git a/src/coreclr/vm/exinfo.h b/src/coreclr/vm/exinfo.h index 302975c5d7ec04..534671f396191b 100644 --- a/src/coreclr/vm/exinfo.h +++ b/src/coreclr/vm/exinfo.h @@ -268,6 +268,7 @@ struct ExInfo // Never, ever destroy a preallocated exception handle. if ((m_hThrowable != NULL) && !CLRException::IsPreallocatedExceptionHandle(m_hThrowable)) { + GCX_COOP(); DestroyHandle(m_hThrowable); } diff --git a/src/coreclr/vm/gchandleutilities.h b/src/coreclr/vm/gchandleutilities.h index f6f08499222206..19a537504a92e6 100644 --- a/src/coreclr/vm/gchandleutilities.h +++ b/src/coreclr/vm/gchandleutilities.h @@ -247,7 +247,7 @@ inline void DestroyHandleCommon(OBJECTHANDLE handle, HandleType type) { NOTHROW; GC_NOTRIGGER; - MODE_ANY; + MODE_COOPERATIVE; CAN_TAKE_LOCK; } CONTRACTL_END; diff --git a/src/coreclr/vm/interoplibinterface_comwrappers.cpp b/src/coreclr/vm/interoplibinterface_comwrappers.cpp index 7b984171ff6715..9e8f9ee4c25caf 100644 --- a/src/coreclr/vm/interoplibinterface_comwrappers.cpp +++ b/src/coreclr/vm/interoplibinterface_comwrappers.cpp @@ -267,6 +267,7 @@ namespace InteropLibImports CONTRACTL_END; ::OBJECTHANDLE objectHandle = static_cast<::OBJECTHANDLE>(handle); + GCX_COOP(); DestroyRefcountedHandle(objectHandle); } diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index ed9d728ee88748..60773e8671dc57 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -392,6 +392,7 @@ class CEEInfo : public ICorJitInfo // Free all handles used by JIT if (m_pJitHandles != nullptr) { + GCX_COOP(); OBJECTHANDLE* elements = m_pJitHandles->GetElements(); unsigned count = m_pJitHandles->GetCount(); for (unsigned i = 0; i < count; i++) diff --git a/src/coreclr/vm/syncblk.cpp b/src/coreclr/vm/syncblk.cpp index c210092c685fdb..3c7c4e20d43ce1 100644 --- a/src/coreclr/vm/syncblk.cpp +++ b/src/coreclr/vm/syncblk.cpp @@ -789,6 +789,7 @@ void SyncBlockCache::DeleteSyncBlock(SyncBlock *psb) psb->m_thinLock.StoreWithoutBarrier(0); if (psb->m_Lock) { + GCX_COOP(); DestroyHandle(psb->m_Lock); psb->m_Lock = NULL; } diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index e51ca6c5bd8667..1b1032eb761374 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2297,6 +2297,7 @@ Thread::~Thread() // Destroy any handles that we're using to hold onto exception objects SafeSetThrowables(NULL); + GCX_COOP(); DestroyShortWeakHandle(m_ExposedObject); DestroyStrongHandle(m_StrongHndToExposedObject); } From ad930c00dbfd2f6e9245b5056873ecb2e0fc3c59 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 10 Mar 2026 00:08:51 +0100 Subject: [PATCH 02/13] address feedback --- src/coreclr/debug/ee/debugger.cpp | 11 ++++++++--- src/coreclr/debug/ee/debugger.h | 5 ++++- .../vm/interoplibinterface_comwrappers.cpp | 15 +++++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 1285520e62af25..39d29c2a85c442 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -10984,7 +10984,12 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) OBJECTHANDLE objectHandle = pEvent->DisposeHandle.vmObjectHandle.GetRawPtr(); CorDebugHandleType handleType = pEvent->DisposeHandle.handleType; - GCX_COOP(); + // Switch to cooperative mode if a managed thread exists. + // On the native debugger helper thread (no managed Thread), + // the IFTHREAD variant is a no-op and the MODE_COOPERATIVE + // contract in DestroyHandleCommon is also a no-op. + GCX_COOP_EEINTERFACE_IFTHREAD(); + switch (handleType) { case HANDLE_STRONG: @@ -12305,7 +12310,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF } else { - GCX_COOP(); + GCX_COOP_EEINTERFACE_IFTHREAD(); DestroyLongWeakHandle(objHandle); } } @@ -12315,7 +12320,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF { m_pForceCatchHandlerFoundEventsTable->Remove(objHandle); } - GCX_COOP(); + GCX_COOP_EEINTERFACE_IFTHREAD(); DestroyLongWeakHandle(objHandle); } return S_OK; diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index fd7be4ff3a7d45..f546640accea8a 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -299,6 +299,9 @@ class GCHolderEEInterface #define GCX_COOP_EEINTERFACE() \ GCHolderEEInterface __gcCoop_onlyOneAllowedPerScope +#define GCX_COOP_EEINTERFACE_IFTHREAD() \ + GCHolderEEInterface __gcCoop_onlyOneAllowedPerScope + #define GCX_PREEMP_EEINTERFACE() \ GCHolderEEInterface __gcCoop_onlyOneAllowedPerScope @@ -535,7 +538,7 @@ class EMPTY_BASES_DECL ForceCatchHandlerFoundSHashTraits : public DefaultSHashTr } static void OnRemovePerEntryCleanupAction(const OBJECTHANDLE &e) { - GCX_COOP(); + GCX_COOP_EEINTERFACE_IFTHREAD(); DestroyLongWeakHandle(e); } }; diff --git a/src/coreclr/vm/interoplibinterface_comwrappers.cpp b/src/coreclr/vm/interoplibinterface_comwrappers.cpp index 9e8f9ee4c25caf..a3768bf297759e 100644 --- a/src/coreclr/vm/interoplibinterface_comwrappers.cpp +++ b/src/coreclr/vm/interoplibinterface_comwrappers.cpp @@ -267,8 +267,19 @@ namespace InteropLibImports CONTRACTL_END; ::OBJECTHANDLE objectHandle = static_cast<::OBJECTHANDLE>(handle); - GCX_COOP(); - DestroyRefcountedHandle(objectHandle); + // This can be called from threads without a managed Thread (e.g. via + // COM release on a native thread). Only switch to cooperative mode + // when a managed thread exists; otherwise the MODE_COOPERATIVE + // contract check in DestroyHandleCommon is a no-op. + if (GetThreadNULLOk() != nullptr) + { + GCX_COOP(); + DestroyRefcountedHandle(objectHandle); + } + else + { + DestroyRefcountedHandle(objectHandle); + } } bool GetGlobalPeggingState() noexcept From f72dfcef2b0a952a9d18e9cc68d95fc42d9a9c90 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 10 Mar 2026 03:11:42 +0100 Subject: [PATCH 03/13] Feedback --- src/coreclr/vm/eedbginterface.h | 2 -- src/coreclr/vm/eedbginterfaceimpl.cpp | 26 -------------------------- src/coreclr/vm/eedbginterfaceimpl.h | 3 --- src/coreclr/vm/syncblk.cpp | 3 +-- 4 files changed, 1 insertion(+), 33 deletions(-) diff --git a/src/coreclr/vm/eedbginterface.h b/src/coreclr/vm/eedbginterface.h index a6fd0182fd5a19..81863857aa4fa4 100644 --- a/src/coreclr/vm/eedbginterface.h +++ b/src/coreclr/vm/eedbginterface.h @@ -96,8 +96,6 @@ class EEDebugInterface bool fStrongNewRef, AppDomain *pAppDomain) = 0; - virtual void DbgDestroyHandle( OBJECTHANDLE oh, bool fStrongNewRef ) = 0; - virtual OBJECTHANDLE GetThreadException(Thread *pThread) = 0; virtual bool IsThreadExceptionNull(Thread *pThread) = 0; diff --git a/src/coreclr/vm/eedbginterfaceimpl.cpp b/src/coreclr/vm/eedbginterfaceimpl.cpp index a39598769860ca..041200005d3009 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.cpp +++ b/src/coreclr/vm/eedbginterfaceimpl.cpp @@ -205,32 +205,6 @@ OBJECTHANDLE EEDbgInterfaceImpl::GetHandleFromObject(void *obj, return oh; } -void EEDbgInterfaceImpl::DbgDestroyHandle(OBJECTHANDLE oh, - bool fStrongNewRef) -{ - CONTRACTL - { - NOTHROW; - MODE_ANY; - GC_NOTRIGGER; - } - CONTRACTL_END; - - LOG((LF_CORDB, LL_INFO1000, "EEI::GHFO: Destroyed given handle 0x%x," - "fStrong: 0x%x!\n", oh, fStrongNewRef)); - - GCX_COOP(); - if (fStrongNewRef) - { - DestroyStrongHandle(oh); - } - else - { - DestroyLongWeakHandle(oh); - } -} - - OBJECTHANDLE EEDbgInterfaceImpl::GetThreadException(Thread *pThread) { CONTRACTL diff --git a/src/coreclr/vm/eedbginterfaceimpl.h b/src/coreclr/vm/eedbginterfaceimpl.h index 3e59d15da56ab3..a329d258e72202 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.h +++ b/src/coreclr/vm/eedbginterfaceimpl.h @@ -83,9 +83,6 @@ class EEDbgInterfaceImpl : public EEDebugInterface bool fStrongNewRef, AppDomain *pAppDomain); - void DbgDestroyHandle(OBJECTHANDLE oh, - bool fStrongNewRef); - OBJECTHANDLE GetThreadException(Thread *pThread); bool IsThreadExceptionNull(Thread *pThread); diff --git a/src/coreclr/vm/syncblk.cpp b/src/coreclr/vm/syncblk.cpp index 3c7c4e20d43ce1..994d154a59a651 100644 --- a/src/coreclr/vm/syncblk.cpp +++ b/src/coreclr/vm/syncblk.cpp @@ -764,7 +764,7 @@ void SyncBlockCache::DeleteSyncBlock(SyncBlock *psb) INSTANCE_CHECK; THROWS; GC_TRIGGERS; - MODE_ANY; + MODE_COOPERATIVE; INJECT_FAULT(COMPlusThrowOM()); } CONTRACTL_END; @@ -789,7 +789,6 @@ void SyncBlockCache::DeleteSyncBlock(SyncBlock *psb) psb->m_thinLock.StoreWithoutBarrier(0); if (psb->m_Lock) { - GCX_COOP(); DestroyHandle(psb->m_Lock); psb->m_Lock = NULL; } From 926e258f2a89d3d30b0c5619e361ae1a70c79026 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 10 Mar 2026 03:14:17 +0100 Subject: [PATCH 04/13] Feedback --- src/coreclr/vm/encee.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index 72c012dedef4d7..30571e09938728 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -1469,13 +1469,12 @@ void EnCSyncBlockInfo::Cleanup() { NOTHROW; GC_NOTRIGGER; - MODE_ANY; + MODE_COOPERATIVE; } CONTRACTL_END; // Walk our linked list of all the fields that were added EnCAddedField *pEntry = m_pList; { - GCX_COOP(); while (pEntry) { // Clean up the handle we created in EnCAddedField::Allocate From 1248f0b72c03150f5f296ce1c466c5aad6658663 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 11 Mar 2026 02:20:19 +0100 Subject: [PATCH 05/13] will this work? --- src/coreclr/vm/appdomain.cpp | 11 ++++++++-- src/coreclr/vm/clrex.cpp | 11 ++++++++-- src/coreclr/vm/comcallablewrapper.cpp | 11 ++++++++-- src/coreclr/vm/comconnectionpoints.h | 11 ++++++++-- src/coreclr/vm/dllimportcallback.cpp | 11 ++++++++-- src/coreclr/vm/gchandleutilities.h | 22 +++++++++++++++++++ .../vm/interoplibinterface_comwrappers.cpp | 7 +++--- src/coreclr/vm/threads.cpp | 8 ++++--- 8 files changed, 75 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index be594538d52af1..bc3c8f72c1a73b 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -136,8 +136,15 @@ PinnedHeapHandleBucket::~PinnedHeapHandleBucket() if (m_hndHandleArray) { - GCX_COOP(); - DestroyStrongHandle(m_hndHandleArray); + if (GetThreadNULLOk() != NULL) + { + GCX_COOP(); + DestroyStrongHandle(m_hndHandleArray); + } + else + { + DestroyHandleUnsafe(m_hndHandleArray, HNDTYPE_STRONG); + } m_hndHandleArray = NULL; } } diff --git a/src/coreclr/vm/clrex.cpp b/src/coreclr/vm/clrex.cpp index 6f45897b7fe7b7..f09b3a8df5a83f 100644 --- a/src/coreclr/vm/clrex.cpp +++ b/src/coreclr/vm/clrex.cpp @@ -51,8 +51,15 @@ CLRException::~CLRException() // clear the handle first, so if we SO on destroying it, we don't have a dangling reference SetThrowableHandle(NULL); - GCX_COOP(); - DestroyHandle(throwableHandle); + if (GetThreadNULLOk() != NULL) + { + GCX_COOP(); + DestroyHandle(throwableHandle); + } + else + { + DestroyHandleUnsafe(throwableHandle, HNDTYPE_DEFAULT); + } } } diff --git a/src/coreclr/vm/comcallablewrapper.cpp b/src/coreclr/vm/comcallablewrapper.cpp index d9555016582542..a052276dc6336d 100644 --- a/src/coreclr/vm/comcallablewrapper.cpp +++ b/src/coreclr/vm/comcallablewrapper.cpp @@ -2014,8 +2014,15 @@ void ComCallWrapper::ClearHandle() OBJECTHANDLE pThis = m_ppThis; if (InterlockedCompareExchangeT(&m_ppThis, NULL, pThis) == pThis) { - GCX_COOP(); - DestroyRefcountedHandle(pThis); + if (GetThreadNULLOk() != NULL) + { + GCX_COOP(); + DestroyRefcountedHandle(pThis); + } + else + { + DestroyHandleUnsafe(pThis, HNDTYPE_REFCOUNTED); + } } } diff --git a/src/coreclr/vm/comconnectionpoints.h b/src/coreclr/vm/comconnectionpoints.h index fc777f89430b11..f1b9e1e6e859ec 100644 --- a/src/coreclr/vm/comconnectionpoints.h +++ b/src/coreclr/vm/comconnectionpoints.h @@ -50,8 +50,15 @@ struct ConnectionCookie ~ConnectionCookie() { WRAPPER_NO_CONTRACT; - GCX_COOP(); - DestroyHandle(m_hndEventProvObj); + if (GetThreadNULLOk() != NULL) + { + GCX_COOP(); + DestroyHandle(m_hndEventProvObj); + } + else + { + DestroyHandleUnsafe(m_hndEventProvObj, HNDTYPE_DEFAULT); + } } // Currently called only from Cooperative mode. diff --git a/src/coreclr/vm/dllimportcallback.cpp b/src/coreclr/vm/dllimportcallback.cpp index ffc4b15737e1eb..0228570ea8ae3a 100644 --- a/src/coreclr/vm/dllimportcallback.cpp +++ b/src/coreclr/vm/dllimportcallback.cpp @@ -319,8 +319,15 @@ void UMEntryThunkData::Terminate() if (pObjectHandle != NULL) { - GCX_COOP(); - DestroyLongWeakHandle(pObjectHandle); + if (GetThreadNULLOk() != NULL) + { + GCX_COOP(); + DestroyLongWeakHandle(pObjectHandle); + } + else + { + DestroyHandleUnsafe(pObjectHandle, HNDTYPE_WEAK_LONG); + } } s_thunkFreeList.AddToList(this); diff --git a/src/coreclr/vm/gchandleutilities.h b/src/coreclr/vm/gchandleutilities.h index 19a537504a92e6..93b363b1b0cfc7 100644 --- a/src/coreclr/vm/gchandleutilities.h +++ b/src/coreclr/vm/gchandleutilities.h @@ -256,6 +256,28 @@ inline void DestroyHandleCommon(OBJECTHANDLE handle, HandleType type) GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, type); } +// Variant of DestroyHandleCommon that is safe to call from preemptive mode +// and from threads that the runtime does not know about. +// It nulls the handle value first to prevent GC from seeing stale references, +// skips the profiler notification (no managed thread context), then recycles +// the handle slot through the GC handle manager. +inline void DestroyHandleUnsafe(OBJECTHANDLE handle, HandleType type) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + CAN_TAKE_LOCK; + } + CONTRACTL_END; + + // Null the handle value first so that GC will not follow a stale + // object reference while we recycle the slot. + GCHandleUtilities::GetGCHandleManager()->StoreObjectInHandle(handle, NULL); + GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, type); +} + inline void DestroyHandle(OBJECTHANDLE handle) { DestroyHandleCommon(handle, HNDTYPE_DEFAULT); diff --git a/src/coreclr/vm/interoplibinterface_comwrappers.cpp b/src/coreclr/vm/interoplibinterface_comwrappers.cpp index a3768bf297759e..468a9bccc3212a 100644 --- a/src/coreclr/vm/interoplibinterface_comwrappers.cpp +++ b/src/coreclr/vm/interoplibinterface_comwrappers.cpp @@ -268,9 +268,8 @@ namespace InteropLibImports ::OBJECTHANDLE objectHandle = static_cast<::OBJECTHANDLE>(handle); // This can be called from threads without a managed Thread (e.g. via - // COM release on a native thread). Only switch to cooperative mode - // when a managed thread exists; otherwise the MODE_COOPERATIVE - // contract check in DestroyHandleCommon is a no-op. + // COM release on a native thread). Use the unsafe variant that nulls + // the handle before recycling when we cannot enter cooperative mode. if (GetThreadNULLOk() != nullptr) { GCX_COOP(); @@ -278,7 +277,7 @@ namespace InteropLibImports } else { - DestroyRefcountedHandle(objectHandle); + DestroyHandleUnsafe(objectHandle, HNDTYPE_REFCOUNTED); } } diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 1b1032eb761374..2e4486cbe1df13 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2297,9 +2297,11 @@ Thread::~Thread() // Destroy any handles that we're using to hold onto exception objects SafeSetThrowables(NULL); - GCX_COOP(); - DestroyShortWeakHandle(m_ExposedObject); - DestroyStrongHandle(m_StrongHndToExposedObject); + // The thread is already detached from TLS (GetThread() returns NULL), + // so GCX_COOP() cannot be used. Use DestroyHandleUnsafe which nulls + // the handle value before recycling the slot. + DestroyHandleUnsafe(m_ExposedObject, HNDTYPE_WEAK_SHORT); + DestroyHandleUnsafe(m_StrongHndToExposedObject, HNDTYPE_STRONG); } g_pThinLockThreadIdDispenser->DisposeId(GetThreadId()); From e5128fd475837c26334db49bd6aa41c886d7368b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 11 Mar 2026 04:09:52 +0100 Subject: [PATCH 06/13] Add HndDestroyHandleLocked --- src/coreclr/gc/gchandletable.cpp | 5 ++++ src/coreclr/gc/gchandletableimpl.h | 2 ++ src/coreclr/gc/gcinterface.h | 4 +++ src/coreclr/gc/handletable.cpp | 47 ++++++++++++++++++++++++++++++ src/coreclr/gc/handletable.h | 1 + src/coreclr/vm/gchandleutilities.h | 10 ++----- 6 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/coreclr/gc/gchandletable.cpp b/src/coreclr/gc/gchandletable.cpp index d503aca269b6cd..8f4c14671fbfb7 100644 --- a/src/coreclr/gc/gchandletable.cpp +++ b/src/coreclr/gc/gchandletable.cpp @@ -170,6 +170,11 @@ void GCHandleManager::DestroyHandleOfType(OBJECTHANDLE handle, HandleType type) ::HndDestroyHandle(::HndGetHandleTable(handle), type, handle); } +void GCHandleManager::DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type) +{ + ::HndDestroyHandleLocked(::HndGetHandleTable(handle), type, handle); +} + void GCHandleManager::DestroyHandleOfUnknownType(OBJECTHANDLE handle) { ::HndDestroyHandleOfUnknownType(::HndGetHandleTable(handle), handle); diff --git a/src/coreclr/gc/gchandletableimpl.h b/src/coreclr/gc/gchandletableimpl.h index 83bcab2b8fd8dd..cdeedac694511c 100644 --- a/src/coreclr/gc/gchandletableimpl.h +++ b/src/coreclr/gc/gchandletableimpl.h @@ -51,6 +51,8 @@ class GCHandleManager : public IGCHandleManager virtual void DestroyHandleOfType(OBJECTHANDLE handle, HandleType type); + virtual void DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type); + virtual void DestroyHandleOfUnknownType(OBJECTHANDLE handle); virtual void SetExtraInfoForHandle(OBJECTHANDLE handle, HandleType type, void* pExtraInfo); diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h index 43bd1752cc4168..82b93eb87bce89 100644 --- a/src/coreclr/gc/gcinterface.h +++ b/src/coreclr/gc/gcinterface.h @@ -619,6 +619,10 @@ class IGCHandleManager { virtual void DestroyHandleOfType(OBJECTHANDLE handle, HandleType type) PURE_VIRTUAL + // Variant of DestroyHandleOfType that takes the handle table lock. + // Safe to call from preemptive mode or from threads not known to the runtime. + virtual void DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type) PURE_VIRTUAL + virtual void DestroyHandleOfUnknownType(OBJECTHANDLE handle) PURE_VIRTUAL virtual void SetExtraInfoForHandle(OBJECTHANDLE handle, HandleType type, void* pExtraInfo) PURE_VIRTUAL diff --git a/src/coreclr/gc/handletable.cpp b/src/coreclr/gc/handletable.cpp index 356b0e0bdc314b..48bb99a0a1aa70 100644 --- a/src/coreclr/gc/handletable.cpp +++ b/src/coreclr/gc/handletable.cpp @@ -396,6 +396,53 @@ void HndDestroyHandle(HHANDLETABLE hTable, uint32_t uType, OBJECTHANDLE handle) } +/* + * HndDestroyHandleLocked + * + * Entrypoint for freeing an individual handle under the handle table lock. + * Safe to call from preemptive mode or from threads not known to the runtime + * because the lock prevents races with GC scanning. + * + */ +void HndDestroyHandleLocked(HHANDLETABLE hTable, uint32_t uType, OBJECTHANDLE handle) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + CAN_TAKE_LOCK; + } + CONTRACTL_END; + + STRESS_LOG2(LF_GC, LL_INFO1000, "DestroyHandleLocked: *%p->%p\n", handle, *(_UNCHECKED_OBJECTREF *)handle); + + FIRE_EVENT(DestroyGCHandle, (void *)handle); + FIRE_EVENT(PrvDestroyGCHandle, (void *)handle); + + // sanity check handle we are being asked to free + _ASSERTE(handle); + + // fetch the handle table pointer + HandleTable *pTable = Table(hTable); + + // sanity check the type index + _ASSERTE(uType < pTable->uTypeCount); + + _ASSERTE(HandleFetchType(handle) == uType); + + // Take the handle table lock to prevent races with GC scanning. + CrstHolder ch(&pTable->Lock); + + // return the handle to the table's cache (under lock) + TableFreeSingleHandleToCache(pTable, uType, handle); + +#if defined(ENABLE_PERF_COUNTERS) || defined(FEATURE_EVENT_TRACE) + g_dwHandles--; +#endif // defined(ENABLE_PERF_COUNTERS) || defined(FEATURE_EVENT_TRACE) +} + + /* * HndDestroyHandleOfUnknownType * diff --git a/src/coreclr/gc/handletable.h b/src/coreclr/gc/handletable.h index 4bd6aa1a7bbd6b..9568e4f3f8d556 100644 --- a/src/coreclr/gc/handletable.h +++ b/src/coreclr/gc/handletable.h @@ -68,6 +68,7 @@ uint32_t HndGetHandleTableIndex(HHANDLETABLE hTable); */ OBJECTHANDLE HndCreateHandle(HHANDLETABLE hTable, uint32_t uType, OBJECTREF object, uintptr_t lExtraInfo = 0); void HndDestroyHandle(HHANDLETABLE hTable, uint32_t uType, OBJECTHANDLE handle); +void HndDestroyHandleLocked(HHANDLETABLE hTable, uint32_t uType, OBJECTHANDLE handle); void HndDestroyHandleOfUnknownType(HHANDLETABLE hTable, OBJECTHANDLE handle); diff --git a/src/coreclr/vm/gchandleutilities.h b/src/coreclr/vm/gchandleutilities.h index 93b363b1b0cfc7..b5ef604330b4a8 100644 --- a/src/coreclr/vm/gchandleutilities.h +++ b/src/coreclr/vm/gchandleutilities.h @@ -258,9 +258,8 @@ inline void DestroyHandleCommon(OBJECTHANDLE handle, HandleType type) // Variant of DestroyHandleCommon that is safe to call from preemptive mode // and from threads that the runtime does not know about. -// It nulls the handle value first to prevent GC from seeing stale references, -// skips the profiler notification (no managed thread context), then recycles -// the handle slot through the GC handle manager. +// It takes the handle table lock to prevent races with GC scanning, +// bypassing the lock-free cache path. inline void DestroyHandleUnsafe(OBJECTHANDLE handle, HandleType type) { CONTRACTL @@ -272,10 +271,7 @@ inline void DestroyHandleUnsafe(OBJECTHANDLE handle, HandleType type) } CONTRACTL_END; - // Null the handle value first so that GC will not follow a stale - // object reference while we recycle the slot. - GCHandleUtilities::GetGCHandleManager()->StoreObjectInHandle(handle, NULL); - GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, type); + GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfTypeLocked(handle, type); } inline void DestroyHandle(OBJECTHANDLE handle) From 65812e9e5de5928f99d808c50e4d86201308a2c5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 11 Mar 2026 23:09:58 +0100 Subject: [PATCH 07/13] feedback --- src/coreclr/gc/handletable.cpp | 2 +- src/coreclr/vm/appdomain.cpp | 2 +- src/coreclr/vm/clrex.cpp | 2 +- src/coreclr/vm/comcallablewrapper.cpp | 2 +- src/coreclr/vm/comconnectionpoints.h | 2 +- src/coreclr/vm/dllimportcallback.cpp | 2 +- src/coreclr/vm/gchandleutilities.h | 3 ++- .../vm/interoplibinterface_comwrappers.cpp | 2 +- src/coreclr/vm/threads.cpp | 15 +++++++++------ 9 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/coreclr/gc/handletable.cpp b/src/coreclr/gc/handletable.cpp index 48bb99a0a1aa70..6e159d47d90526 100644 --- a/src/coreclr/gc/handletable.cpp +++ b/src/coreclr/gc/handletable.cpp @@ -415,7 +415,7 @@ void HndDestroyHandleLocked(HHANDLETABLE hTable, uint32_t uType, OBJECTHANDLE ha } CONTRACTL_END; - STRESS_LOG2(LF_GC, LL_INFO1000, "DestroyHandleLocked: *%p->%p\n", handle, *(_UNCHECKED_OBJECTREF *)handle); + STRESS_LOG1(LF_GC, LL_INFO1000, "DestroyHandleLocked: %p\n", handle); FIRE_EVENT(DestroyGCHandle, (void *)handle); FIRE_EVENT(PrvDestroyGCHandle, (void *)handle); diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index bc3c8f72c1a73b..4ae7ad899c849e 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -143,7 +143,7 @@ PinnedHeapHandleBucket::~PinnedHeapHandleBucket() } else { - DestroyHandleUnsafe(m_hndHandleArray, HNDTYPE_STRONG); + DestroyHandleInPreemptiveMode(m_hndHandleArray, HNDTYPE_STRONG); } m_hndHandleArray = NULL; } diff --git a/src/coreclr/vm/clrex.cpp b/src/coreclr/vm/clrex.cpp index f09b3a8df5a83f..47135c89f96b62 100644 --- a/src/coreclr/vm/clrex.cpp +++ b/src/coreclr/vm/clrex.cpp @@ -58,7 +58,7 @@ CLRException::~CLRException() } else { - DestroyHandleUnsafe(throwableHandle, HNDTYPE_DEFAULT); + DestroyHandleInPreemptiveMode(throwableHandle, HNDTYPE_DEFAULT); } } } diff --git a/src/coreclr/vm/comcallablewrapper.cpp b/src/coreclr/vm/comcallablewrapper.cpp index a052276dc6336d..56ddf647a86b10 100644 --- a/src/coreclr/vm/comcallablewrapper.cpp +++ b/src/coreclr/vm/comcallablewrapper.cpp @@ -2021,7 +2021,7 @@ void ComCallWrapper::ClearHandle() } else { - DestroyHandleUnsafe(pThis, HNDTYPE_REFCOUNTED); + DestroyHandleInPreemptiveMode(pThis, HNDTYPE_REFCOUNTED); } } } diff --git a/src/coreclr/vm/comconnectionpoints.h b/src/coreclr/vm/comconnectionpoints.h index f1b9e1e6e859ec..ecaa841d0e5823 100644 --- a/src/coreclr/vm/comconnectionpoints.h +++ b/src/coreclr/vm/comconnectionpoints.h @@ -57,7 +57,7 @@ struct ConnectionCookie } else { - DestroyHandleUnsafe(m_hndEventProvObj, HNDTYPE_DEFAULT); + DestroyHandleInPreemptiveMode(m_hndEventProvObj, HNDTYPE_DEFAULT); } } diff --git a/src/coreclr/vm/dllimportcallback.cpp b/src/coreclr/vm/dllimportcallback.cpp index 0228570ea8ae3a..1b13bd51db8b44 100644 --- a/src/coreclr/vm/dllimportcallback.cpp +++ b/src/coreclr/vm/dllimportcallback.cpp @@ -326,7 +326,7 @@ void UMEntryThunkData::Terminate() } else { - DestroyHandleUnsafe(pObjectHandle, HNDTYPE_WEAK_LONG); + DestroyHandleInPreemptiveMode(pObjectHandle, HNDTYPE_WEAK_LONG); } } diff --git a/src/coreclr/vm/gchandleutilities.h b/src/coreclr/vm/gchandleutilities.h index b5ef604330b4a8..d7d71afc9293b1 100644 --- a/src/coreclr/vm/gchandleutilities.h +++ b/src/coreclr/vm/gchandleutilities.h @@ -260,7 +260,7 @@ inline void DestroyHandleCommon(OBJECTHANDLE handle, HandleType type) // and from threads that the runtime does not know about. // It takes the handle table lock to prevent races with GC scanning, // bypassing the lock-free cache path. -inline void DestroyHandleUnsafe(OBJECTHANDLE handle, HandleType type) +inline void DestroyHandleInPreemptiveMode(OBJECTHANDLE handle, HandleType type) { CONTRACTL { @@ -271,6 +271,7 @@ inline void DestroyHandleUnsafe(OBJECTHANDLE handle, HandleType type) } CONTRACTL_END; + DiagHandleDestroyed(handle); GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfTypeLocked(handle, type); } diff --git a/src/coreclr/vm/interoplibinterface_comwrappers.cpp b/src/coreclr/vm/interoplibinterface_comwrappers.cpp index 468a9bccc3212a..92d668eeda89b7 100644 --- a/src/coreclr/vm/interoplibinterface_comwrappers.cpp +++ b/src/coreclr/vm/interoplibinterface_comwrappers.cpp @@ -277,7 +277,7 @@ namespace InteropLibImports } else { - DestroyHandleUnsafe(objectHandle, HNDTYPE_REFCOUNTED); + DestroyHandleInPreemptiveMode(objectHandle, HNDTYPE_REFCOUNTED); } } diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 2e4486cbe1df13..5bb1731a4853c8 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2296,12 +2296,6 @@ Thread::~Thread() { // Destroy any handles that we're using to hold onto exception objects SafeSetThrowables(NULL); - - // The thread is already detached from TLS (GetThread() returns NULL), - // so GCX_COOP() cannot be used. Use DestroyHandleUnsafe which nulls - // the handle value before recycling the slot. - DestroyHandleUnsafe(m_ExposedObject, HNDTYPE_WEAK_SHORT); - DestroyHandleUnsafe(m_StrongHndToExposedObject, HNDTYPE_STRONG); } g_pThinLockThreadIdDispenser->DisposeId(GetThreadId()); @@ -2588,6 +2582,15 @@ void Thread::CooperativeCleanup() OBJECTREF threadObjMaybe = GetExposedObjectRaw(); if (threadObjMaybe != NULL) ((THREADBASEREF)threadObjMaybe)->SetIsDead(); + + // Destroy handles that we're using to hold onto exception objects and the exposed thread object. + // This runs under GCX_COOP() above, so it is safe to destroy handles here. + SafeSetThrowables(NULL); + + DestroyShortWeakHandle(m_ExposedObject); + m_ExposedObject = NULL; + DestroyStrongHandle(m_StrongHndToExposedObject); + m_StrongHndToExposedObject = NULL; } // See general comments on thread destruction (code:#threadDestruction) above. From e072c5e8ce047d3f8d01b5aa0a1f62e113ea3ec1 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 24 Mar 2026 17:14:45 +0100 Subject: [PATCH 08/13] Address review feedback: GC interface version, thread destructor safety, debugger paths - Move DestroyHandleOfTypeLocked to end of IGCHandleManager for non-breaking GC interface compatibility; bump GC_INTERFACE_MINOR_VERSION 8 -> 9 - Replace SafeSetThrowables(NULL) in ~Thread() with direct preemptive-safe handle destruction for m_LastThrownObjectHandle, m_ExposedObject, and m_StrongHndToExposedObject to avoid calling DestroyHandle outside COOP mode and prevent handle leaks when CooperativeCleanup is skipped - Use DestroyHandleInPreemptiveMode in debugger DisposeHandle, UpdateForceCatchHandlerFoundTable, and OnRemovePerEntryCleanupAction since these can run on the helper thread with no managed Thread - Remove unused GCX_COOP_EEINTERFACE_IFTHREAD macro - Fix misleading comment on DestroyHandleInPreemptiveMode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 20 +++++++------------- src/coreclr/debug/ee/debugger.h | 6 +----- src/coreclr/gc/gchandletable.cpp | 10 +++++----- src/coreclr/gc/gchandletableimpl.h | 4 ++-- src/coreclr/gc/gcinterface.h | 10 +++++----- src/coreclr/vm/gchandleutilities.h | 4 ++-- src/coreclr/vm/threads.cpp | 22 ++++++++++++++++++++-- 7 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 39d29c2a85c442..351ae6ac24368d 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -10984,22 +10984,18 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) OBJECTHANDLE objectHandle = pEvent->DisposeHandle.vmObjectHandle.GetRawPtr(); CorDebugHandleType handleType = pEvent->DisposeHandle.handleType; - // Switch to cooperative mode if a managed thread exists. - // On the native debugger helper thread (no managed Thread), - // the IFTHREAD variant is a no-op and the MODE_COOPERATIVE - // contract in DestroyHandleCommon is also a no-op. - GCX_COOP_EEINTERFACE_IFTHREAD(); - + // Use the preemptive-mode-safe variant because this can run on + // the native debugger helper thread which has no managed Thread. switch (handleType) { case HANDLE_STRONG: - DestroyStrongHandle(objectHandle); + DestroyHandleInPreemptiveMode(objectHandle, HNDTYPE_STRONG); break; case HANDLE_WEAK_TRACK_RESURRECTION: - DestroyLongWeakHandle(objectHandle); + DestroyHandleInPreemptiveMode(objectHandle, HNDTYPE_WEAK_LONG); break; case HANDLE_PINNED: - DestroyPinningHandle(objectHandle); + DestroyHandleInPreemptiveMode(objectHandle, HNDTYPE_PINNED); break; default: pEvent->hr = E_INVALIDARG; @@ -12310,8 +12306,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF } else { - GCX_COOP_EEINTERFACE_IFTHREAD(); - DestroyLongWeakHandle(objHandle); + DestroyHandleInPreemptiveMode(objHandle, HNDTYPE_WEAK_LONG); } } else @@ -12320,8 +12315,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF { m_pForceCatchHandlerFoundEventsTable->Remove(objHandle); } - GCX_COOP_EEINTERFACE_IFTHREAD(); - DestroyLongWeakHandle(objHandle); + DestroyHandleInPreemptiveMode(objHandle, HNDTYPE_WEAK_LONG); } return S_OK; } diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index f546640accea8a..0f24aceab7113c 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -299,9 +299,6 @@ class GCHolderEEInterface #define GCX_COOP_EEINTERFACE() \ GCHolderEEInterface __gcCoop_onlyOneAllowedPerScope -#define GCX_COOP_EEINTERFACE_IFTHREAD() \ - GCHolderEEInterface __gcCoop_onlyOneAllowedPerScope - #define GCX_PREEMP_EEINTERFACE() \ GCHolderEEInterface __gcCoop_onlyOneAllowedPerScope @@ -538,8 +535,7 @@ class EMPTY_BASES_DECL ForceCatchHandlerFoundSHashTraits : public DefaultSHashTr } static void OnRemovePerEntryCleanupAction(const OBJECTHANDLE &e) { - GCX_COOP_EEINTERFACE_IFTHREAD(); - DestroyLongWeakHandle(e); + DestroyHandleInPreemptiveMode(e, HNDTYPE_WEAK_LONG); } }; typedef SHash ForceCatchHandlerFoundTable; diff --git a/src/coreclr/gc/gchandletable.cpp b/src/coreclr/gc/gchandletable.cpp index 8f4c14671fbfb7..6574d103b3554f 100644 --- a/src/coreclr/gc/gchandletable.cpp +++ b/src/coreclr/gc/gchandletable.cpp @@ -170,11 +170,6 @@ void GCHandleManager::DestroyHandleOfType(OBJECTHANDLE handle, HandleType type) ::HndDestroyHandle(::HndGetHandleTable(handle), type, handle); } -void GCHandleManager::DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type) -{ - ::HndDestroyHandleLocked(::HndGetHandleTable(handle), type, handle); -} - void GCHandleManager::DestroyHandleOfUnknownType(OBJECTHANDLE handle) { ::HndDestroyHandleOfUnknownType(::HndGetHandleTable(handle), handle); @@ -227,3 +222,8 @@ void GCHandleManager::TraceRefCountedHandles(HANDLESCANPROC callback, uintptr_t ::Ref_TraceRefCountHandles(callback, param1, param2); } +void GCHandleManager::DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type) +{ + ::HndDestroyHandleLocked(::HndGetHandleTable(handle), type, handle); +} + diff --git a/src/coreclr/gc/gchandletableimpl.h b/src/coreclr/gc/gchandletableimpl.h index cdeedac694511c..a0eea7fc212e44 100644 --- a/src/coreclr/gc/gchandletableimpl.h +++ b/src/coreclr/gc/gchandletableimpl.h @@ -51,8 +51,6 @@ class GCHandleManager : public IGCHandleManager virtual void DestroyHandleOfType(OBJECTHANDLE handle, HandleType type); - virtual void DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type); - virtual void DestroyHandleOfUnknownType(OBJECTHANDLE handle); virtual void SetExtraInfoForHandle(OBJECTHANDLE handle, HandleType type, void* pExtraInfo); @@ -72,6 +70,8 @@ class GCHandleManager : public IGCHandleManager virtual HandleType HandleFetchType(OBJECTHANDLE handle); virtual void TraceRefCountedHandles(HANDLESCANPROC callback, uintptr_t param1, uintptr_t param2); + + virtual void DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type); }; #endif // GCHANDLETABLE_H_ diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h index 82b93eb87bce89..4862c0a1363ef7 100644 --- a/src/coreclr/gc/gcinterface.h +++ b/src/coreclr/gc/gcinterface.h @@ -11,7 +11,7 @@ // The minor version of the IGCHeap interface. Non-breaking changes are required // to bump the minor version number. GCs and EEs with minor version number // mismatches can still interoperate correctly, with some care. -#define GC_INTERFACE_MINOR_VERSION 8 +#define GC_INTERFACE_MINOR_VERSION 9 // The major version of the IGCToCLR interface. Breaking changes to this interface // require bumps in the major version number. @@ -619,10 +619,6 @@ class IGCHandleManager { virtual void DestroyHandleOfType(OBJECTHANDLE handle, HandleType type) PURE_VIRTUAL - // Variant of DestroyHandleOfType that takes the handle table lock. - // Safe to call from preemptive mode or from threads not known to the runtime. - virtual void DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type) PURE_VIRTUAL - virtual void DestroyHandleOfUnknownType(OBJECTHANDLE handle) PURE_VIRTUAL virtual void SetExtraInfoForHandle(OBJECTHANDLE handle, HandleType type, void* pExtraInfo) PURE_VIRTUAL @@ -642,6 +638,10 @@ class IGCHandleManager { virtual HandleType HandleFetchType(OBJECTHANDLE handle) PURE_VIRTUAL virtual void TraceRefCountedHandles(HANDLESCANPROC callback, uintptr_t param1, uintptr_t param2) PURE_VIRTUAL + + // Variant of DestroyHandleOfType that takes the handle table lock. + // Safe to call from preemptive mode or from threads not known to the runtime. + virtual void DestroyHandleOfTypeLocked(OBJECTHANDLE handle, HandleType type) PURE_VIRTUAL }; // Enum representing the type to be passed to GC.CoreCLR.cs used to deduce the type of configuration. diff --git a/src/coreclr/vm/gchandleutilities.h b/src/coreclr/vm/gchandleutilities.h index d7d71afc9293b1..7ecb87b1bc147a 100644 --- a/src/coreclr/vm/gchandleutilities.h +++ b/src/coreclr/vm/gchandleutilities.h @@ -258,8 +258,8 @@ inline void DestroyHandleCommon(OBJECTHANDLE handle, HandleType type) // Variant of DestroyHandleCommon that is safe to call from preemptive mode // and from threads that the runtime does not know about. -// It takes the handle table lock to prevent races with GC scanning, -// bypassing the lock-free cache path. +// It takes the handle table lock to synchronize with GC handle scanning, +// preventing races that would occur on the lock-free cache path. inline void DestroyHandleInPreemptiveMode(OBJECTHANDLE handle, HandleType type) { CONTRACTL diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index e68441e4e1ff53..16da1b1cd71cf5 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2292,8 +2292,26 @@ Thread::~Thread() if (!IsAtProcessExit()) { - // Destroy any handles that we're using to hold onto exception objects - SafeSetThrowables(NULL); + // Destroy any remaining handles that weren't cleaned up by CooperativeCleanup. + // Use DestroyHandleInPreemptiveMode since the destructor can run outside + // cooperative mode (e.g., if HasStarted failed and CooperativeCleanup was skipped). + if (m_LastThrownObjectHandle != NULL && + !CLRException::IsPreallocatedExceptionHandle(m_LastThrownObjectHandle)) + { + DestroyHandleInPreemptiveMode(m_LastThrownObjectHandle, HNDTYPE_DEFAULT); + m_LastThrownObjectHandle = NULL; + } + + if (m_ExposedObject != NULL) + { + DestroyHandleInPreemptiveMode(m_ExposedObject, HNDTYPE_WEAK_SHORT); + m_ExposedObject = NULL; + } + if (m_StrongHndToExposedObject != NULL) + { + DestroyHandleInPreemptiveMode(m_StrongHndToExposedObject, HNDTYPE_STRONG); + m_StrongHndToExposedObject = NULL; + } } g_pThinLockThreadIdDispenser->DisposeId(GetThreadId()); From e0f0d9bb6b292d699f039bad4529387b8d68fef5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 25 Mar 2026 13:20:51 +0100 Subject: [PATCH 09/13] Fix AV in DecExternalCount when m_StrongHndToExposedObject is already destroyed CooperativeCleanup (called from OnThreadTerminate) now destroys m_StrongHndToExposedObject and sets it to NULL. DecExternalCount, called later in OnThreadTerminate, dereferences the handle without a NULL check, causing an access violation. Add a NULL guard before the dereference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/threads.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 16da1b1cd71cf5..897ade9d6478d1 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2173,7 +2173,9 @@ int Thread::DecExternalCount(BOOL holdingLock) // that is holding a reference to us. To make sure that we are not the // ones keeping the exposed object alive we need to remove the strong // reference we have to it. - if ((retVal == 1) && ((*((void**)m_StrongHndToExposedObject)) != NULL)) + // m_StrongHndToExposedObject may already be NULL if CooperativeCleanup + // destroyed it earlier during OnThreadTerminate. + if ((retVal == 1) && (m_StrongHndToExposedObject != NULL) && ((*((void**)m_StrongHndToExposedObject)) != NULL)) { // Switch back to cooperative mode to manipulate the object. From 1aaf9dce43787e56c7ec67b5b7dc8b15232771e5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 25 Mar 2026 15:05:38 +0100 Subject: [PATCH 10/13] Address review feedback: fix race in DecExternalCount, fix contract in CEEInfo - DecExternalCount: Replace racy StoreObjectInHandle(NULL) in preemptive mode with DestroyHandleInPreemptiveMode which takes the handle table lock, preventing GC from overwriting the NULL with a relocated value. - CEEInfo::~CEEInfo: Replace LIMITED_METHOD_CONTRACT with WRAPPER_NO_CONTRACT since GCX_COOP() switches GC modes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/jitinterface.h | 6 +++++- src/coreclr/vm/threads.cpp | 17 ++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 60773e8671dc57..2c61fffa3b85f9 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -386,7 +386,11 @@ class CEEInfo : public ICorJitInfo virtual ~CEEInfo() { - LIMITED_METHOD_CONTRACT; + CONTRACTL { + NOTHROW; + GC_TRIGGERS; + } + CONTRACTL_END; #if !defined(DACCESS_COMPILE) // Free all handles used by JIT diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 897ade9d6478d1..1ed7cbc40f8bb1 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2177,17 +2177,10 @@ int Thread::DecExternalCount(BOOL holdingLock) // destroyed it earlier during OnThreadTerminate. if ((retVal == 1) && (m_StrongHndToExposedObject != NULL) && ((*((void**)m_StrongHndToExposedObject)) != NULL)) { - // Switch back to cooperative mode to manipulate the object. - - // Don't want to switch back to COOP until we let go of the lock - // however we are allowed to call StoreObjectInHandle here in preemptive - // mode because we are setting the value to NULL. - CONTRACT_VIOLATION(ModeViolation); - - // Clear the handle and leave the lock. - // We do not have to DisablePreemptiveGC here, because - // we just want to put NULL into a handle. - StoreObjectInHandle(m_StrongHndToExposedObject, NULL); + // Destroy the strong handle under the handle table lock to avoid + // racing with GC relocating the handle value in preemptive mode. + DestroyHandleInPreemptiveMode(m_StrongHndToExposedObject, HNDTYPE_STRONG); + m_StrongHndToExposedObject = NULL; tsLock.Release(); @@ -2197,8 +2190,6 @@ int Thread::DecExternalCount(BOOL holdingLock) pCurThread->DisablePreemptiveGC(); } - GCX_ASSERT_COOP(); - return retVal; } } From 3d57fa4ea08212a64e11f8081decd0c10c820d84 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 26 Mar 2026 01:32:26 +0100 Subject: [PATCH 11/13] Fix assert in ObjectFromHandle after CooperativeCleanup destroys exposed object handles CooperativeCleanup() was destroying m_ExposedObject and m_StrongHndToExposedObject during DetachThread(), but these handles are still needed afterwards by CleanupDetachedThreads() which calls IncExternalCount() (dereferences the handle directly), GetExposedObject() (calls ObjectFromHandle), and the managed OnThreadExiting callback. ShouldTriggerGCForDeadThreads() also calls GetExposedObjectRaw() on dead threads. Remove the early handle destruction from CooperativeCleanup. The handles are safely destroyed later in ~Thread() via DestroyHandleInPreemptiveMode (locked path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/threads.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 1ed7cbc40f8bb1..194d4ff91b2d98 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2593,14 +2593,15 @@ void Thread::CooperativeCleanup() if (threadObjMaybe != NULL) ((THREADBASEREF)threadObjMaybe)->SetIsDead(); - // Destroy handles that we're using to hold onto exception objects and the exposed thread object. + // Destroy handles that we're using to hold onto exception objects. // This runs under GCX_COOP() above, so it is safe to destroy handles here. SafeSetThrowables(NULL); - DestroyShortWeakHandle(m_ExposedObject); - m_ExposedObject = NULL; - DestroyStrongHandle(m_StrongHndToExposedObject); - m_StrongHndToExposedObject = NULL; + // Note: m_ExposedObject and m_StrongHndToExposedObject are intentionally + // NOT destroyed here. They are still needed by CleanupDetachedThreads() + // which calls IncExternalCount(), GetExposedObject(), and the managed + // OnThreadExiting callback after the thread has detached. + // These handles are destroyed later in ~Thread() via DestroyHandleInPreemptiveMode. } // See general comments on thread destruction (code:#threadDestruction) above. From 016cc3a4a35948096f0facee2d1543d8984d9c41 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 26 Mar 2026 17:47:27 +0100 Subject: [PATCH 12/13] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/exinfo.cpp | 12 ++++++++++-- src/coreclr/vm/exinfo.h | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/exinfo.cpp b/src/coreclr/vm/exinfo.cpp index cbde40416bfa51..8e840a757fac2a 100644 --- a/src/coreclr/vm/exinfo.cpp +++ b/src/coreclr/vm/exinfo.cpp @@ -76,8 +76,16 @@ void ExInfo::ReleaseResources() { if (!CLRException::IsPreallocatedExceptionHandle(m_hThrowable)) { - GCX_COOP(); - DestroyHandle(m_hThrowable); + Thread *pThread = GetThreadNULLOk(); + if (pThread != NULL) + { + GCX_COOP(); + DestroyHandle(m_hThrowable); + } + else + { + DestroyHandleInPreemptiveMode(m_hThrowable, HNDTYPE_DEFAULT); + } } m_hThrowable = NULL; } diff --git a/src/coreclr/vm/exinfo.h b/src/coreclr/vm/exinfo.h index 534671f396191b..dac2749a4745b1 100644 --- a/src/coreclr/vm/exinfo.h +++ b/src/coreclr/vm/exinfo.h @@ -268,8 +268,16 @@ struct ExInfo // Never, ever destroy a preallocated exception handle. if ((m_hThrowable != NULL) && !CLRException::IsPreallocatedExceptionHandle(m_hThrowable)) { - GCX_COOP(); - DestroyHandle(m_hThrowable); + Thread *pThread = GetThreadNULLOk(); + if (pThread != NULL) + { + GCX_COOP(); + DestroyHandle(m_hThrowable); + } + else + { + DestroyHandleInPreemptiveMode(m_hThrowable, HNDTYPE_DEFAULT); + } } m_hThrowable = NULL; From a04100af28ae070ce9ddf06b09fa2e555655565f Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 3 Jun 2026 16:42:44 +0200 Subject: [PATCH 13/13] Remove stale comment about CooperativeCleanup destroying handles The comment claimed m_StrongHndToExposedObject could be NULL because CooperativeCleanup destroyed it, but commit 3d57fa4ea08 explicitly moved that destruction back to ~Thread(). The defensive NULL guard is kept as-is; only the misleading comment is removed. --- src/coreclr/vm/threads.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index eacc8504d59e3a..0de6b1ca409fe2 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -2145,8 +2145,6 @@ int Thread::DecExternalCount(BOOL holdingLock) // that is holding a reference to us. To make sure that we are not the // ones keeping the exposed object alive we need to remove the strong // reference we have to it. - // m_StrongHndToExposedObject may already be NULL if CooperativeCleanup - // destroyed it earlier during OnThreadTerminate. if ((retVal == 1) && (m_StrongHndToExposedObject != NULL) && ((*((void**)m_StrongHndToExposedObject)) != NULL)) { // Destroy the strong handle under the handle table lock to avoid