Delete GC handles in COOP mode

src/coreclr/binder/customassemblybinder.cpp

@@ -258,6 +258,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<OBJECTHANDLE>(GetAssemblyLoadContext());
+
+    GCX_COOP();
     DestroyLongWeakHandle(handle);
     handle = reinterpret_cast<OBJECTHANDLE>(m_ptrManagedStrongAssemblyLoadContext);
     DestroyHandle(handle);

src/coreclr/debug/ee/debugger.cpp

@@ -10830,16 +10830,18 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
             OBJECTHANDLE objectHandle = pEvent->DisposeHandle.vmObjectHandle.GetRawPtr();
             CorDebugHandleType handleType = pEvent->DisposeHandle.handleType;
 
+            // 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;
@@ -12148,7 +12150,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF
         }
         else
         {
-            DestroyLongWeakHandle(objHandle);
+            DestroyHandleInPreemptiveMode(objHandle, HNDTYPE_WEAK_LONG);
         }
     }
     else
@@ -12157,7 +12159,7 @@ HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF
         {
             m_pForceCatchHandlerFoundEventsTable->Remove(objHandle);
         }
-        DestroyLongWeakHandle(objHandle);
+        DestroyHandleInPreemptiveMode(objHandle, HNDTYPE_WEAK_LONG);
     }
     return S_OK;
 }

src/coreclr/debug/ee/debugger.h

@@ -511,7 +511,7 @@ class EMPTY_BASES ForceCatchHandlerFoundSHashTraits : public DefaultSHashTraits<
         }
         static void OnRemovePerEntryCleanupAction(const OBJECTHANDLE &e)
         {
-            DestroyLongWeakHandle(e);
+            DestroyHandleInPreemptiveMode(e, HNDTYPE_WEAK_LONG);
         }
 };
 typedef SHash<ForceCatchHandlerFoundSHashTraits> ForceCatchHandlerFoundTable;

src/coreclr/gc/gchandletable.cpp

@@ -211,3 +211,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);
+}

src/coreclr/gc/gchandletableimpl.h

@@ -70,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_

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.
@@ -638,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.

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_LOG1(LF_GC, LL_INFO1000, "DestroyHandleLocked: %p\n", 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
  *

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);
 

src/coreclr/vm/appdomain.cpp

@@ -136,7 +136,15 @@ PinnedHeapHandleBucket::~PinnedHeapHandleBucket()
 
     if (m_hndHandleArray)
     {
-        DestroyStrongHandle(m_hndHandleArray);
+        if (GetThreadNULLOk() != NULL)
+        {
+            GCX_COOP();
+            DestroyStrongHandle(m_hndHandleArray);
+        }
+        else
+        {
+            DestroyHandleInPreemptiveMode(m_hndHandleArray, HNDTYPE_STRONG);
+        }
         m_hndHandleArray = NULL;
     }
 }

src/coreclr/vm/clrex.cpp

@@ -50,7 +50,16 @@ 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);
-        DestroyHandle(throwableHandle);
+
+        if (GetThreadNULLOk() != NULL)
+        {
+            GCX_COOP();
+            DestroyHandle(throwableHandle);
+        }
+        else
+        {
+            DestroyHandleInPreemptiveMode(throwableHandle, HNDTYPE_DEFAULT);
+        }
     }
 }
 

src/coreclr/vm/comcallablewrapper.cpp

@@ -1803,7 +1803,15 @@ void ComCallWrapper::ClearHandle()
     OBJECTHANDLE pThis = m_ppThis;
     if (InterlockedCompareExchangeT(&m_ppThis, NULL, pThis) == pThis)
     {
-        DestroyRefcountedHandle(pThis);
+        if (GetThreadNULLOk() != NULL)
+        {
+            GCX_COOP();
+            DestroyRefcountedHandle(pThis);
+        }
+        else
+        {
+            DestroyHandleInPreemptiveMode(pThis, HNDTYPE_REFCOUNTED);
+        }
     }
 }
 

src/coreclr/vm/dllimportcallback.cpp

@@ -347,7 +347,15 @@ void UMEntryThunkData::Terminate()
 
     if (pObjectHandle != NULL)
     {
-        DestroyLongWeakHandle(pObjectHandle);
+        if (GetThreadNULLOk() != NULL)
+        {
+            GCX_COOP();
+            DestroyLongWeakHandle(pObjectHandle);
+        }
+        else
+        {
+            DestroyHandleInPreemptiveMode(pObjectHandle, HNDTYPE_WEAK_LONG);
+        }
     }
 
     s_thunkFreeList.AddToList(this);

src/coreclr/vm/dynamicmethod.cpp

@@ -1109,6 +1109,7 @@ void LCGMethodResolver::DestroyResolver()
 
     if (m_managedResolver)
     {
+        GCX_COOP();
         ::DestroyLongWeakHandle(m_managedResolver);
         m_managedResolver = NULL;
     }

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;

src/coreclr/vm/eedbginterfaceimpl.cpp

@@ -205,30 +205,6 @@ OBJECTHANDLE EEDbgInterfaceImpl::GetHandleFromObject(void *obj,
     return oh;
 }
 
-void EEDbgInterfaceImpl::DbgDestroyHandle(OBJECTHANDLE oh,
-                                          bool fStrongNewRef)
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-    }
-    CONTRACTL_END;
-
-    LOG((LF_CORDB, LL_INFO1000, "EEI::GHFO: Destroyed given handle 0x%x,"
-        "fStrong: 0x%x!\n", oh, fStrongNewRef));
-
-    if (fStrongNewRef)
-    {
-        DestroyStrongHandle(oh);
-    }
-    else
-    {
-        DestroyLongWeakHandle(oh);
-    }
-}
-
-
 OBJECTHANDLE EEDbgInterfaceImpl::GetThreadException(Thread *pThread)
 {
     CONTRACTL

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);

src/coreclr/vm/encee.cpp

@@ -1469,20 +1469,22 @@ 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;
-    while (pEntry)
     {
-        // Clean up the handle we created in EnCAddedField::Allocate
-        DestroyDependentHandle(*(OBJECTHANDLE*)&pEntry->m_FieldData);
+        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

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;
@@ -256,6 +256,25 @@ 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 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
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+        MODE_ANY;
+        CAN_TAKE_LOCK;
+    }
+    CONTRACTL_END;
+
+    DiagHandleDestroyed(handle);
+    GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfTypeLocked(handle, type);
+}
+
 inline void DestroyHandle(OBJECTHANDLE handle)
 {
     DestroyHandleCommon(handle, HNDTYPE_DEFAULT);

src/coreclr/vm/interoplibinterface_comwrappers.cpp

@@ -236,7 +236,18 @@ namespace InteropLibImports
         CONTRACTL_END;
         ::OBJECTHANDLE objectHandle = static_cast<::OBJECTHANDLE>(handle);
 
-        DestroyRefcountedHandle(objectHandle);
+        // This can be called from threads without a managed Thread (e.g. via
+        // 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();
+            DestroyRefcountedHandle(objectHandle);
+        }
+        else
+        {
+            DestroyHandleInPreemptiveMode(objectHandle, HNDTYPE_REFCOUNTED);
+        }
     }
 
     bool GetGlobalPeggingState() noexcept

src/coreclr/vm/jitinterface.h

@@ -376,12 +376,17 @@ 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
         if (m_pJitHandles != nullptr)
         {
+            GCX_COOP();
             OBJECTHANDLE* elements = m_pJitHandles->GetElements();
             unsigned count = m_pJitHandles->GetCount();
             for (unsigned i = 0; i < count; i++)

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;