From 8e22d277a5ccd728ffd7e2134bef3d7d215e82dc Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 6 Aug 2026 18:46:43 +0300 Subject: [PATCH 1/4] Reduce weak ref blocking with java interop Bridge objects live in 2 worlds, .net and java so a .net bridge object has a correpsonding java peer. Collection of these objects is triggered by .net. When the .net peer is eligible for collection we build some graph over the set of dead objects and pass it over to java. Java triggers its own collection, collecting the java peers if they are dead as well. .NET android reports which bridge objects died on the java side so we can drop the gchandles for them. This will finally allow .net peers to die in the following collection (since they had to be promoted, given we don't know yet if java peers need to keep them alive or not). Currently obtaining the target of a weak ref blocks until the bridge processing is fully completed. This is the case also on mono and prevents 2 issues: - normal c# code checks a weak ref for some object. This can't immediately return correct information. If it returns true, the object gets resurrected and we can end up with a ref to a bridge objects that no longer has a java peer. If it returns false then that can be false as well if the object remains alive. - java code could call into managed, inserting a reference to a C# peer and afterward it could drop its own java peer. If this happens while C# gc ran but the java gc is still yet to start, both GC would see their peer as dead, even though it is alive. The .NET android interop obtains the C# peer ref also via weak reference, so this safely synchronizes with bridge processing. This PR keeps the weak reference wait only for bridge objects that are currently processed. For a weak ref target we need to determine whether the underlying object is pending bridge processing which is awkward to do efficiently because we would need to iterate over a set of handles or implement a lookup from obj address to associated cross reference handle. It turns out there is a free bit in the object header that we could use for this purpose. FIXME this has a race with redudndant bridge processing, because a new collection would dirty our g_registeredBridgeHandles. --- src/coreclr/gc/env/gcenv.object.h | 3 ++ src/coreclr/gc/gcbridge.cpp | 19 ++++++++ src/coreclr/gc/gcbridge.h | 2 + src/coreclr/gc/objecthandle.cpp | 3 ++ src/coreclr/nativeaot/Runtime/ObjectLayout.h | 3 ++ .../Runtime/interoplibinterface_java.cpp | 27 +++++++++++- src/coreclr/vm/interoplibinterface.h | 4 ++ src/coreclr/vm/interoplibinterface_java.cpp | 44 ++++++++++++++++++- src/coreclr/vm/marshalnative.cpp | 5 +-- src/coreclr/vm/syncblk.h | 9 ++-- 10 files changed, 109 insertions(+), 10 deletions(-) diff --git a/src/coreclr/gc/env/gcenv.object.h b/src/coreclr/gc/env/gcenv.object.h index 1a7077c75722a0..683ee3996c3943 100644 --- a/src/coreclr/gc/env/gcenv.object.h +++ b/src/coreclr/gc/env/gcenv.object.h @@ -21,6 +21,9 @@ extern bool g_oldMethodTableFlags; // Bits stolen from the sync block index that the GC/HandleTable knows about (currently these are at the same // positions as the mainline runtime but we can change this below when it becomes apparent how NativeAOT will // handle sync blocks). +#ifdef FEATURE_JAVAMARSHAL +#define BIT_SBLK_BRIDGE_PENDING 0x80000000 +#endif // FEATURE_JAVAMARSHAL #define BIT_SBLK_GC_RESERVE 0x20000000 #define BIT_SBLK_FINALIZER_RUN 0x40000000 diff --git a/src/coreclr/gc/gcbridge.cpp b/src/coreclr/gc/gcbridge.cpp index 31cc595922aa0c..0dd7cd005ddf91 100644 --- a/src/coreclr/gc/gcbridge.cpp +++ b/src/coreclr/gc/gcbridge.cpp @@ -261,6 +261,7 @@ static DynPtrArray g_scanStack, g_loopStack; // Objects from crossref handles registered with RegisterBridgeObject static DynPtrArray g_registeredBridges; static DynPtrArray g_registeredBridgesContexts; +static DynPtrArray g_registeredBridgeHandles; // As we traverse the graph, which ColorData objects are accessible from our current position? static DynPtrArray g_colorMergeArray; @@ -1090,6 +1091,7 @@ void BridgeResetData() { DynPtrArrayEmpty(&g_registeredBridges); DynPtrArrayEmpty(&g_registeredBridgesContexts); + DynPtrArrayEmpty(&g_registeredBridgeHandles); DynPtrArrayEmpty(&g_scanStack); DynPtrArrayEmpty(&g_loopStack); EmptyObjectBuckets(); @@ -1105,6 +1107,17 @@ void RegisterBridgeObject(Object* object, uintptr_t context) DynPtrArrayAdd(&g_registeredBridgesContexts, (void*)context); } +void RegisterPendingBridgeHandle(uintptr_t handle) +{ + DynPtrArrayAdd(&g_registeredBridgeHandles, (void*)handle); +} + +uintptr_t* GetPendingBridgeHandles(size_t* count) +{ + *count = (size_t)DynPtrArraySize(&g_registeredBridgeHandles); + return (uintptr_t*)g_registeredBridgeHandles.data; +} + uint8_t** GetRegisteredBridges(size_t* pNumBridges) { *pNumBridges = (size_t)g_registeredBridges.size; @@ -1298,6 +1311,12 @@ MarkCrossReferencesArgs* ProcessBridgeObjects() ResetObjectsHeader(); + for (int i = 0; i < DynPtrArraySize(&g_registeredBridges); i++) + { + Object* object = (Object*)DynPtrArrayGet(&g_registeredBridges, i); + object->GetHeader()->SetBit(BIT_SBLK_BRIDGE_PENDING); + } + BridgeFinish(); return args; diff --git a/src/coreclr/gc/gcbridge.h b/src/coreclr/gc/gcbridge.h index a61388f64392f9..dd6038954886b4 100644 --- a/src/coreclr/gc/gcbridge.h +++ b/src/coreclr/gc/gcbridge.h @@ -18,6 +18,8 @@ MarkCrossReferencesArgs* ProcessBridgeObjects(); bool ShouldProcessBridgeObjects(); void RegisterBridgeObject(Object *object, uintptr_t context); +void RegisterPendingBridgeHandle(uintptr_t handle); +uintptr_t* GetPendingBridgeHandles(size_t* count); uint8_t** GetRegisteredBridges(size_t *pNumBridges); #endif // FEATURE_JAVAMARSHAL diff --git a/src/coreclr/gc/objecthandle.cpp b/src/coreclr/gc/objecthandle.cpp index 10ebc01485ba3d..91eda85a8d691d 100644 --- a/src/coreclr/gc/objecthandle.cpp +++ b/src/coreclr/gc/objecthandle.cpp @@ -1504,6 +1504,7 @@ void CALLBACK GetBridgeObjectsForProcessing(_UNCHECKED_OBJECTREF* pObjRef, uintp if (!g_theGCHeap->IsPromoted(*ppRef)) { RegisterBridgeObject(*ppRef, *pExtraInfo); + RegisterPendingBridgeHandle((uintptr_t)pObjRef); } } @@ -1515,6 +1516,8 @@ uint8_t** Ref_ScanBridgeObjects(uint32_t condemned, uint32_t maxgen, ScanContext uint32_t flags = HNDGCF_NORMAL; uint32_t type = HNDTYPE_CROSSREFERENCE; + // FIXME: Skip rebuilding bridge data while client bridge processing from a previous GC is active. + // The pending bridge handle array is borrowed by the interop layer until that processing completes. BridgeResetData(); HandleTableMap* walk = &g_HandleTableMap; diff --git a/src/coreclr/nativeaot/Runtime/ObjectLayout.h b/src/coreclr/nativeaot/Runtime/ObjectLayout.h index 4c477272a72e77..46bf4d70531cef 100644 --- a/src/coreclr/nativeaot/Runtime/ObjectLayout.h +++ b/src/coreclr/nativeaot/Runtime/ObjectLayout.h @@ -7,6 +7,9 @@ // Bits stolen from the sync block index that the GC/HandleTable knows about (currently these are at the same // positions as the mainline runtime). +#ifdef FEATURE_JAVAMARSHAL +#define BIT_SBLK_BRIDGE_PENDING 0x80000000 +#endif // FEATURE_JAVAMARSHAL #define BIT_SBLK_GC_RESERVE 0x20000000 #define BIT_SBLK_FINALIZER_RUN 0x40000000 diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp index 54aee68af28527..74975139195353 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp @@ -15,6 +15,7 @@ #include "event.h" #include "thread.inl" +#include "gcbridge.h" #include "interoplibinterface.h" using CrossreferenceHandleCallback = void(__stdcall *)(MarkCrossReferencesArgs*); @@ -26,6 +27,19 @@ namespace Volatile g_GCBridgeActive = false; CLREventStatic g_bridgeFinished; + void ClearPendingBridgeBits( + uintptr_t* handles, + size_t handleCount) + { + for (size_t i = 0; i < handleCount; i++) + { + OBJECTHANDLE handle = reinterpret_cast(handles[i]); + Object* object = ObjectFromHandle(handle); + if (object != nullptr) + object->GetHeader()->ClrBit(BIT_SBLK_BRIDGE_PENDING); + } + } + void ReleaseGCBridgeArgumentsWorker( MarkCrossReferencesArgs* args) { @@ -55,8 +69,12 @@ void JavaMarshalNative::TriggerClientBridgeProcessing( { _ASSERTE(GCHeapUtilities::IsGCInProgress()); + size_t pendingBridgeHandleCount; + uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + if (g_GCBridgeActive) { + // FIXME: This should become unreachable once bridge graph recomputation is skipped while active. // Release the memory allocated since the GCBridge // is already running and we're not passing them to it. ReleaseGCBridgeArgumentsWorker(args); @@ -68,6 +86,7 @@ void JavaMarshalNative::TriggerClientBridgeProcessing( { // Release the memory allocated since we // don't have a GC bridge callback. + ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount); ReleaseGCBridgeArgumentsWorker(args); return; } @@ -122,12 +141,14 @@ extern "C" void QCALLTYPE JavaMarshal_FinishCrossReferenceProcessing( pThisThread->DisablePreemptiveMode(); GCHeapUtilities::GetGCHeap()->NullBridgeObjectsWeakRefs(length, unreachableObjectHandles); + size_t pendingBridgeHandleCount; + uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount); IGCHandleManager* pHandleManager = GCHandleUtilities::GetGCHandleManager(); OBJECTHANDLE* handles = (OBJECTHANDLE*)unreachableObjectHandles; for (size_t i = 0; i < length; i++) pHandleManager->DestroyHandleOfUnknownType(handles[i]); - g_GCBridgeActive = false; g_bridgeFinished.Set(); @@ -139,7 +160,9 @@ extern "C" void QCALLTYPE JavaMarshal_FinishCrossReferenceProcessing( FCIMPL2(FC_BOOL_RET, GCHandle_InternalTryGetBridgeWait, OBJECTHANDLE handle, OBJECTREF* pObjResult) { - if (g_GCBridgeActive) + Object* object = ObjectFromHandle(handle); + if (g_GCBridgeActive && object != nullptr && + (object->GetHeader()->GetBits() & BIT_SBLK_BRIDGE_PENDING) != 0) { FC_RETURN_BOOL(false); } diff --git a/src/coreclr/vm/interoplibinterface.h b/src/coreclr/vm/interoplibinterface.h index c9da0f7eaeeef3..23838ed77009f3 100644 --- a/src/coreclr/vm/interoplibinterface.h +++ b/src/coreclr/vm/interoplibinterface.h @@ -150,6 +150,10 @@ class Interop static bool IsGCBridgeActive(); + static bool TryGetObjectFromHandleWithoutBridgeWait( + _In_ OBJECTHANDLE handle, + _Out_ Object** result); + static void WaitForGCBridgeFinish(); static void TriggerClientBridgeProcessing( diff --git a/src/coreclr/vm/interoplibinterface_java.cpp b/src/coreclr/vm/interoplibinterface_java.cpp index f3048be9551ca5..b01b2b8bb4d11e 100644 --- a/src/coreclr/vm/interoplibinterface_java.cpp +++ b/src/coreclr/vm/interoplibinterface_java.cpp @@ -9,6 +9,7 @@ // Interop library header #include +#include "../gc/gcbridge.h" #include "interoplibinterface.h" using CrossreferenceHandleCallback = void(STDMETHODCALLTYPE *)(MarkCrossReferencesArgs*); @@ -20,6 +21,23 @@ namespace Volatile g_GCBridgeActive = false; CLREvent* g_bridgeFinished = nullptr; + void ClearPendingBridgeBits( + _In_reads_(handleCount) uintptr_t* handles, + size_t handleCount) + { + LIMITED_METHOD_CONTRACT; + + for (size_t i = 0; i < handleCount; i++) + { + OBJECTHANDLE handle = reinterpret_cast(handles[i]); + Object* object = OBJECTREFToObject(ObjectFromHandle(handle)); + if (object != nullptr) + { + object->GetHeader()->ClrBit(BIT_SBLK_BRIDGE_PENDING); + } + } + } + void ReleaseGCBridgeArgumentsWorker( _In_ MarkCrossReferencesArgs* args) { @@ -51,6 +69,23 @@ bool Interop::IsGCBridgeActive() return g_GCBridgeActive; } +bool Interop::TryGetObjectFromHandleWithoutBridgeWait( + _In_ OBJECTHANDLE handle, + _Out_ Object** result) +{ + LIMITED_METHOD_CONTRACT; + + Object* object = OBJECTREFToObject(ObjectFromHandle(handle)); + if (g_GCBridgeActive && object != nullptr && + (object->GetHeader()->GetBits() & BIT_SBLK_BRIDGE_PENDING) != 0) + { + return false; + } + + *result = object; + return true; +} + void Interop::WaitForGCBridgeFinish() { CONTRACTL @@ -80,8 +115,12 @@ void Interop::TriggerClientBridgeProcessing( } CONTRACTL_END; + size_t pendingBridgeHandleCount; + uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + if (g_GCBridgeActive) { + // FIXME: This should become unreachable once bridge graph recomputation is skipped while active. // Release the memory allocated since the GCBridge // is already running and we're not passing them to it. ReleaseGCBridgeArgumentsWorker(args); @@ -94,6 +133,7 @@ void Interop::TriggerClientBridgeProcessing( { // Release the memory allocated since the GCBridge // wasn't trigger for some reason. + ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount); ReleaseGCBridgeArgumentsWorker(args); return; } @@ -122,11 +162,13 @@ void Interop::FinishCrossReferenceProcessing( GCX_COOP(); GCHeapUtilities::GetGCHeap()->NullBridgeObjectsWeakRefs(length, unreachableObjectHandles); + size_t pendingBridgeHandleCount; + uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount); IGCHandleManager* pHandleManager = GCHandleUtilities::GetGCHandleManager(); for (size_t i = 0; i < length; i++) pHandleManager->DestroyHandleOfUnknownType(((OBJECTHANDLE*)unreachableObjectHandles)[i]); - g_GCBridgeActive = false; g_bridgeFinished->Set(); } diff --git a/src/coreclr/vm/marshalnative.cpp b/src/coreclr/vm/marshalnative.cpp index 44fbaaac59f847..d1530f14e9fc18 100644 --- a/src/coreclr/vm/marshalnative.cpp +++ b/src/coreclr/vm/marshalnative.cpp @@ -397,12 +397,9 @@ FCIMPL2(FC_BOOL_RET, MarshalNative::GCHandleInternalTryGetBridgeWait, OBJECTHAND { FCALL_CONTRACT; - if (Interop::IsGCBridgeActive()) - { + if (!Interop::TryGetObjectFromHandleWithoutBridgeWait(handle, pObjResult)) FC_RETURN_BOOL(false); - } - *pObjResult = OBJECTREFToObject(ObjectFromHandle(handle)); FC_RETURN_BOOL(true); } FCIMPLEND diff --git a/src/coreclr/vm/syncblk.h b/src/coreclr/vm/syncblk.h index 81dc4f1dec5528..2990a885007b3a 100644 --- a/src/coreclr/vm/syncblk.h +++ b/src/coreclr/vm/syncblk.h @@ -82,11 +82,14 @@ typedef DPTR(EnCSyncBlockInfo) PTR_EnCSyncBlockInfo; // to zero out the ObjHeader for the current allocation. And the limits of the // GC space are initialized to respect this "off by one" error. -// m_SyncBlockValue is carved up into an index and a set of bits. Steal bits by -// reducing the mask. We use the very high bit, in _DEBUG, to be sure we never forget -// to mask the Value to obtain the Index +// m_SyncBlockValue is carved up into an index and a set of bits. Steal bits by +// reducing the mask. +#ifdef FEATURE_JAVAMARSHAL +#define BIT_SBLK_BRIDGE_PENDING 0x80000000 +#else #define BIT_SBLK_UNUSED 0x80000000 +#endif // FEATURE_JAVAMARSHAL #define BIT_SBLK_FINALIZER_RUN 0x40000000 #define BIT_SBLK_GC_RESERVE 0x20000000 From 95d5b3ec2107c3aa5f7762688760363be302822c Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 11 Aug 2026 13:24:22 +0300 Subject: [PATCH 2/4] Fix implementation, now that we no longer do redundant bridge computation --- src/coreclr/gc/gcbridge.cpp | 5 +++-- src/coreclr/gc/gcbridge.h | 2 +- src/coreclr/gc/objecthandle.cpp | 12 ++++++------ .../nativeaot/Runtime/interoplibinterface_java.cpp | 11 ++--------- src/coreclr/vm/interoplibinterface_java.cpp | 9 +-------- 5 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/coreclr/gc/gcbridge.cpp b/src/coreclr/gc/gcbridge.cpp index 0dd7cd005ddf91..c7f4805bdcbf39 100644 --- a/src/coreclr/gc/gcbridge.cpp +++ b/src/coreclr/gc/gcbridge.cpp @@ -1087,11 +1087,12 @@ static void BridgeFinish() #endif } -void BridgeResetData() +void BridgeResetData(bool resetPendingBridgeHandles) { DynPtrArrayEmpty(&g_registeredBridges); DynPtrArrayEmpty(&g_registeredBridgesContexts); - DynPtrArrayEmpty(&g_registeredBridgeHandles); + if (resetPendingBridgeHandles) + DynPtrArrayEmpty(&g_registeredBridgeHandles); DynPtrArrayEmpty(&g_scanStack); DynPtrArrayEmpty(&g_loopStack); EmptyObjectBuckets(); diff --git a/src/coreclr/gc/gcbridge.h b/src/coreclr/gc/gcbridge.h index dd6038954886b4..32400b98397e19 100644 --- a/src/coreclr/gc/gcbridge.h +++ b/src/coreclr/gc/gcbridge.h @@ -9,7 +9,7 @@ #include "common.h" #include "gcinterface.h" -void BridgeResetData(); +void BridgeResetData(bool resetPendingBridgeHandles); MarkCrossReferencesArgs* ProcessBridgeObjects(); // Decides whether this collection should hand a fresh set of cross references to the client. diff --git a/src/coreclr/gc/objecthandle.cpp b/src/coreclr/gc/objecthandle.cpp index 91eda85a8d691d..f948cd1f00fe0e 100644 --- a/src/coreclr/gc/objecthandle.cpp +++ b/src/coreclr/gc/objecthandle.cpp @@ -1504,7 +1504,8 @@ void CALLBACK GetBridgeObjectsForProcessing(_UNCHECKED_OBJECTREF* pObjRef, uintp if (!g_theGCHeap->IsPromoted(*ppRef)) { RegisterBridgeObject(*ppRef, *pExtraInfo); - RegisterPendingBridgeHandle((uintptr_t)pObjRef); + if (lp2 != 0) + RegisterPendingBridgeHandle((uintptr_t)pObjRef); } } @@ -1515,10 +1516,9 @@ uint8_t** Ref_ScanBridgeObjects(uint32_t condemned, uint32_t maxgen, ScanContext LOG((LF_GC | LF_CORPROF, LL_INFO10000, "Building bridge object graphs.\n")); uint32_t flags = HNDGCF_NORMAL; uint32_t type = HNDTYPE_CROSSREFERENCE; + bool shouldProcessBridgeObjects = ShouldProcessBridgeObjects(); - // FIXME: Skip rebuilding bridge data while client bridge processing from a previous GC is active. - // The pending bridge handle array is borrowed by the interop layer until that processing completes. - BridgeResetData(); + BridgeResetData(shouldProcessBridgeObjects); HandleTableMap* walk = &g_HandleTableMap; while (walk) { @@ -1530,14 +1530,14 @@ uint8_t** Ref_ScanBridgeObjects(uint32_t condemned, uint32_t maxgen, ScanContext HHANDLETABLE hTable = walk->pBuckets[i]->pTable[uCPUindex]; if (hTable) // or have a local var for bridgeObjectsToPromote/size (instead of NULL) that's passed in as lp2 - HndScanHandlesForGC(hTable, GetBridgeObjectsForProcessing, uintptr_t(sc), 0, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); + HndScanHandlesForGC(hTable, GetBridgeObjectsForProcessing, uintptr_t(sc), shouldProcessBridgeObjects, &type, 1, condemned, maxgen, HNDGCF_EXTRAINFO | flags); } } walk = walk->pNext; } // The callee here will free the allocated memory. - if (ShouldProcessBridgeObjects()) + if (shouldProcessBridgeObjects) { MarkCrossReferencesArgs *args = ProcessBridgeObjects(); diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp index 74975139195353..f5c27780a71c47 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp @@ -72,14 +72,7 @@ void JavaMarshalNative::TriggerClientBridgeProcessing( size_t pendingBridgeHandleCount; uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); - if (g_GCBridgeActive) - { - // FIXME: This should become unreachable once bridge graph recomputation is skipped while active. - // Release the memory allocated since the GCBridge - // is already running and we're not passing them to it. - ReleaseGCBridgeArgumentsWorker(args); - return; - } + _ASSERTE(!g_GCBridgeActive); // Not initialized if (g_MarkCrossReferences == NULL) @@ -167,7 +160,7 @@ FCIMPL2(FC_BOOL_RET, GCHandle_InternalTryGetBridgeWait, OBJECTHANDLE handle, OBJ FC_RETURN_BOOL(false); } - *pObjResult = ObjectFromHandle(handle); + *pObjResult = object; FC_RETURN_BOOL(true); } FCIMPLEND diff --git a/src/coreclr/vm/interoplibinterface_java.cpp b/src/coreclr/vm/interoplibinterface_java.cpp index b01b2b8bb4d11e..75f2e93e35ce4e 100644 --- a/src/coreclr/vm/interoplibinterface_java.cpp +++ b/src/coreclr/vm/interoplibinterface_java.cpp @@ -118,14 +118,7 @@ void Interop::TriggerClientBridgeProcessing( size_t pendingBridgeHandleCount; uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); - if (g_GCBridgeActive) - { - // FIXME: This should become unreachable once bridge graph recomputation is skipped while active. - // Release the memory allocated since the GCBridge - // is already running and we're not passing them to it. - ReleaseGCBridgeArgumentsWorker(args); - return; - } + _ASSERTE(!g_GCBridgeActive); bool gcBridgeTriggered = JavaNative::TriggerClientBridgeProcessing(args); From 9219d909f19f4ade7f50c480f94f9ba535ed9f4a Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 11 Aug 2026 13:45:51 +0300 Subject: [PATCH 3/4] Move GetPendingBridgeHandles to the GC interface --- src/coreclr/gc/gcimpl.h | 2 ++ src/coreclr/gc/gcinterface.h | 2 ++ src/coreclr/gc/interface.cpp | 12 ++++++++++++ .../nativeaot/Runtime/interoplibinterface_java.cpp | 5 ++--- src/coreclr/vm/interoplibinterface_java.cpp | 5 ++--- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/coreclr/gc/gcimpl.h b/src/coreclr/gc/gcimpl.h index 0ab8741e96fdb3..4fb14a446a2e64 100644 --- a/src/coreclr/gc/gcimpl.h +++ b/src/coreclr/gc/gcimpl.h @@ -334,6 +334,8 @@ class GCHeap : public IGCHeapInternal virtual int RefreshMemoryLimit(); virtual void NullBridgeObjectsWeakRefs(size_t length, void* unreachableObjectHandles); + + virtual uintptr_t* GetPendingBridgeHandles(size_t* count); }; #endif // GCIMPL_H_ diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h index fc27e0cc499136..27d0e1315dd6e8 100644 --- a/src/coreclr/gc/gcinterface.h +++ b/src/coreclr/gc/gcinterface.h @@ -1074,6 +1074,8 @@ class IGCHeap { virtual void DiagWalkHeapWithACHandling(walk_fn fn, void* context, int gen_number, bool walk_large_object_heap_p) PURE_VIRTUAL virtual void NullBridgeObjectsWeakRefs(size_t length, void* unreachableObjectHandles) PURE_VIRTUAL; + + virtual uintptr_t* GetPendingBridgeHandles(size_t* count) PURE_VIRTUAL; }; #ifdef WRITE_BARRIER_CHECK diff --git a/src/coreclr/gc/interface.cpp b/src/coreclr/gc/interface.cpp index 871c84ee737a60..82ce661e176892 100644 --- a/src/coreclr/gc/interface.cpp +++ b/src/coreclr/gc/interface.cpp @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #include "gcinternal.h" +#include "gcbridge.h" #ifdef SERVER_GC namespace SVR @@ -2737,6 +2738,17 @@ void GCHeap::NullBridgeObjectsWeakRefs(size_t length, void* unreachableObjectHan #endif } +uintptr_t* GCHeap::GetPendingBridgeHandles(size_t* count) +{ +#ifdef FEATURE_JAVAMARSHAL + return ::GetPendingBridgeHandles(count); +#else + assert(false); + *count = 0; + return nullptr; +#endif +} + HRESULT GCHeap::WaitUntilConcurrentGCCompleteAsync(int millisecondsTimeout) { #ifdef BACKGROUND_GC diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp index f5c27780a71c47..6c735b9d0e627a 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp @@ -15,7 +15,6 @@ #include "event.h" #include "thread.inl" -#include "gcbridge.h" #include "interoplibinterface.h" using CrossreferenceHandleCallback = void(__stdcall *)(MarkCrossReferencesArgs*); @@ -70,7 +69,7 @@ void JavaMarshalNative::TriggerClientBridgeProcessing( _ASSERTE(GCHeapUtilities::IsGCInProgress()); size_t pendingBridgeHandleCount; - uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + uintptr_t* pendingBridgeHandles = GCHeapUtilities::GetGCHeap()->GetPendingBridgeHandles(&pendingBridgeHandleCount); _ASSERTE(!g_GCBridgeActive); @@ -135,7 +134,7 @@ extern "C" void QCALLTYPE JavaMarshal_FinishCrossReferenceProcessing( GCHeapUtilities::GetGCHeap()->NullBridgeObjectsWeakRefs(length, unreachableObjectHandles); size_t pendingBridgeHandleCount; - uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + uintptr_t* pendingBridgeHandles = GCHeapUtilities::GetGCHeap()->GetPendingBridgeHandles(&pendingBridgeHandleCount); ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount); IGCHandleManager* pHandleManager = GCHandleUtilities::GetGCHandleManager(); diff --git a/src/coreclr/vm/interoplibinterface_java.cpp b/src/coreclr/vm/interoplibinterface_java.cpp index 75f2e93e35ce4e..f9402c9fb21f8d 100644 --- a/src/coreclr/vm/interoplibinterface_java.cpp +++ b/src/coreclr/vm/interoplibinterface_java.cpp @@ -9,7 +9,6 @@ // Interop library header #include -#include "../gc/gcbridge.h" #include "interoplibinterface.h" using CrossreferenceHandleCallback = void(STDMETHODCALLTYPE *)(MarkCrossReferencesArgs*); @@ -116,7 +115,7 @@ void Interop::TriggerClientBridgeProcessing( CONTRACTL_END; size_t pendingBridgeHandleCount; - uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + uintptr_t* pendingBridgeHandles = GCHeapUtilities::GetGCHeap()->GetPendingBridgeHandles(&pendingBridgeHandleCount); _ASSERTE(!g_GCBridgeActive); @@ -156,7 +155,7 @@ void Interop::FinishCrossReferenceProcessing( GCHeapUtilities::GetGCHeap()->NullBridgeObjectsWeakRefs(length, unreachableObjectHandles); size_t pendingBridgeHandleCount; - uintptr_t* pendingBridgeHandles = GetPendingBridgeHandles(&pendingBridgeHandleCount); + uintptr_t* pendingBridgeHandles = GCHeapUtilities::GetGCHeap()->GetPendingBridgeHandles(&pendingBridgeHandleCount); ClearPendingBridgeBits(pendingBridgeHandles, pendingBridgeHandleCount); IGCHandleManager* pHandleManager = GCHandleUtilities::GetGCHandleManager(); From 68531b91722ac1402c36e22fc73bdd65545ad020 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Wed, 12 Aug 2026 19:50:59 +0300 Subject: [PATCH 4/4] Fix race by ensuring we have memory ordering between weakref clearing and clearing of pending bit The bit clearing already had memory ordering since it was done via InterlockedAnd. For the read we add GetBitsAcquire which does an acquire load, preserving the ordering on the reader side. --- src/coreclr/nativeaot/Runtime/ObjectLayout.h | 3 +++ .../Runtime/interoplibinterface_java.cpp | 11 +++++++-- src/coreclr/vm/interoplibinterface_java.cpp | 23 +++++++++++++++++-- src/coreclr/vm/syncblk.h | 8 +++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/ObjectLayout.h b/src/coreclr/nativeaot/Runtime/ObjectLayout.h index 46bf4d70531cef..581af36b2b7006 100644 --- a/src/coreclr/nativeaot/Runtime/ObjectLayout.h +++ b/src/coreclr/nativeaot/Runtime/ObjectLayout.h @@ -5,6 +5,8 @@ // Low-level types describing GC object layouts. // +#include "volatile.h" + // Bits stolen from the sync block index that the GC/HandleTable knows about (currently these are at the same // positions as the mainline runtime). #ifdef FEATURE_JAVAMARSHAL @@ -25,6 +27,7 @@ class ObjHeader public: uint32_t GetBits() { return m_uSyncBlockValue; } + uint32_t GetBitsAcquire() { return VolatileLoad(&m_uSyncBlockValue); } void SetBit(uint32_t uBit); void ClrBit(uint32_t uBit); void SetGCBit() { m_uSyncBlockValue |= BIT_SBLK_GC_RESERVE; } diff --git a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp index 6c735b9d0e627a..f394fa67c185d9 100644 --- a/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp +++ b/src/coreclr/nativeaot/Runtime/interoplibinterface_java.cpp @@ -154,12 +154,19 @@ FCIMPL2(FC_BOOL_RET, GCHandle_InternalTryGetBridgeWait, OBJECTHANDLE handle, OBJ { Object* object = ObjectFromHandle(handle); if (g_GCBridgeActive && object != nullptr && - (object->GetHeader()->GetBits() & BIT_SBLK_BRIDGE_PENDING) != 0) + (object->GetHeader()->GetBitsAcquire() & BIT_SBLK_BRIDGE_PENDING) != 0) { FC_RETURN_BOOL(false); } - *pObjResult = object; + // See explanation in Interop::TryGetObjectFromHandleWithoutBridgeWait + Object* confirmedObject = ObjectFromHandle(handle); + if (confirmedObject != object) + { + FC_RETURN_BOOL(false); + } + + *pObjResult = confirmedObject; FC_RETURN_BOOL(true); } FCIMPLEND diff --git a/src/coreclr/vm/interoplibinterface_java.cpp b/src/coreclr/vm/interoplibinterface_java.cpp index f9402c9fb21f8d..aa5e57e03acc02 100644 --- a/src/coreclr/vm/interoplibinterface_java.cpp +++ b/src/coreclr/vm/interoplibinterface_java.cpp @@ -74,14 +74,33 @@ bool Interop::TryGetObjectFromHandleWithoutBridgeWait( { LIMITED_METHOD_CONTRACT; + // g_GCBridgeActive is Volatile and pending bit clear uses InterlockedAnd. These should be set with release + // stores guaranteeing the order: weak-reference nulling -> pending-bit clearing -> g_GCBridgeActive = false Object* object = OBJECTREFToObject(ObjectFromHandle(handle)); if (g_GCBridgeActive && object != nullptr && - (object->GetHeader()->GetBits() & BIT_SBLK_BRIDGE_PENDING) != 0) + (object->GetHeader()->GetBitsAcquire() & BIT_SBLK_BRIDGE_PENDING) != 0) { return false; } - *result = object; + // If object is nullptr, the handle value is stable, refetching is harmless + // If bridge is not active, refetching the handle should guarantee we get the right value + // since `g_GCBridgeActive` is Volatile + // The only remaining interesting case is gc bridge being active, with non-null object which + // had the bridge pending bit not set. + // + // If the bridge pending bit was never set because the object was not due for collection, handle value is stable + // If the bridge pending bit was set but got cleared in the meantime due to this code racing with the bridge + // finisher, refetching the handle will guarantee that we see the new value of the handle, because the + // bit is cleared with InterlockedAnd which does a release barrier (guaranteeing that the potential handle nulling + // was already published). + Object* confirmedObject = OBJECTREFToObject(ObjectFromHandle(handle)); + if (confirmedObject != object) + { + return false; + } + + *result = confirmedObject; return true; } diff --git a/src/coreclr/vm/syncblk.h b/src/coreclr/vm/syncblk.h index 2990a885007b3a..71a0ffb125bb8c 100644 --- a/src/coreclr/vm/syncblk.h +++ b/src/coreclr/vm/syncblk.h @@ -864,6 +864,14 @@ class ObjHeader return m_SyncBlockValue.LoadWithoutBarrier(); } + DWORD GetBitsAcquire() + { + LIMITED_METHOD_CONTRACT; + SUPPORTS_DAC; + + return m_SyncBlockValue.Load(); + } + DWORD SetBits(DWORD newBits, DWORD oldBits) {