From f4720ee9f1d6ac15a737183056b7ef7bccdc2730 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 18 Mar 2026 15:49:10 -0700 Subject: [PATCH 1/5] Remove native wrappers from the RCW cache in the tracker support global instance when releasing external objects for the Jupiter runtime --- .../Runtime/InteropServices/ComWrappers.cs | 47 +++++++++++++++---- .../InteropServices/TrackerObjectManager.cs | 10 +++- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs index fb2210d780173f..566997f54a7e69 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs @@ -1291,6 +1291,11 @@ private static void AddWrapperToReferenceTrackerHandleCache(NativeObjectWrapper } } + internal void RemoveWrappersFromCache(IEnumerable wrappers) + { + _rcwCache.RemoveAll(wrappers); + } + private sealed class RcwCache { private readonly ReaderWriterLockSlim _lock = new ReaderWriterLockSlim(); @@ -1399,17 +1404,22 @@ public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) _lock.EnterWriteLock(); try { - // TryGetOrCreateObjectForComInstanceInternal may have put a new entry into the cache - // in the time between the GC cleared the contents of the GC handle but before the - // NativeObjectWrapper finalizer ran. - // Only remove the entry if the target of the GC handle is the NativeObjectWrapper - // or is null (indicating that the corresponding NativeObjectWrapper has been scheduled for finalization). - if (_cache.TryGetValue(comPointer, out GCHandle cachedRef) - && (wrapper == cachedRef.Target - || cachedRef.Target is null)) + RemoveLocked(comPointer, wrapper); + } + finally + { + _lock.ExitWriteLock(); + } + } + + public void RemoveAll(IEnumerable wrappers) + { + _lock.EnterWriteLock(); + try + { + foreach (NativeObjectWrapper wrapper in wrappers) { - _cache.Remove(comPointer); - cachedRef.Free(); + RemoveLocked(wrapper.ExternalComObject, wrapper); } } finally @@ -1417,6 +1427,23 @@ public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) _lock.ExitWriteLock(); } } + + private void RemoveLocked(IntPtr comPointer, NativeObjectWrapper wrapper) + { + // This method is used in a scenario where we already have a lock on the cache, so we can skip acquiring the lock again. + // TryGetOrCreateObjectForComInstanceInternal may have put a new entry into the cache + // in the time between the GC cleared the contents of the GC handle but before the + // NativeObjectWrapper finalizer ran. + // Only remove the entry if the target of the GC handle is the NativeObjectWrapper + // or is null (indicating that the corresponding NativeObjectWrapper has been scheduled for finalization). + if (_cache.TryGetValue(comPointer, out GCHandle cachedRef) + && (wrapper == cachedRef.Target + || cachedRef.Target is null)) + { + _cache.Remove(comPointer); + cachedRef.Free(); + } + } } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs index 4243caf0097ff0..f2fa9986b041cf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs @@ -61,7 +61,8 @@ internal static void ReleaseExternalObjectsFromCurrentThread() IntPtr contextToken = GetContextToken(); - List objects = new List(); + List wrappersToRemove = []; + List objects = []; // Here we aren't part of a GC callback, so other threads can still be running // who are adding and removing from the collection. This means we can possibly race @@ -85,8 +86,15 @@ internal static void ReleaseExternalObjectsFromCurrentThread() // Separate the wrapper from the tracker runtime prior to // passing them. nativeObjectWrapper.DisconnectTracker(); + + wrappersToRemove.Add(nativeObjectWrapper); } } + + // Remove the native object wrappers from the cache + // so we don't return released wrappers to the user if the native COM object + // happens to be reused. + GlobalInstanceForTrackerSupport.RemoveWrappersFromCache(wrappersToRemove); } GlobalInstanceForTrackerSupport.ReleaseObjects(objects); From dd82c6047b82c924090115f72da576e4c68ae9ea Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 13:00:25 -0700 Subject: [PATCH 2/5] PR feedback and add a test --- .../Runtime/InteropServices/ComWrappers.cs | 7 +++--- .../InteropServices/TrackerObjectManager.cs | 5 +++- .../GlobalInstance/GlobalInstance.cs | 25 ++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs index 566997f54a7e69..6b4531b2a2cd0f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs @@ -1404,7 +1404,7 @@ public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) _lock.EnterWriteLock(); try { - RemoveLocked(comPointer, wrapper); + Remove_Locked(comPointer, wrapper); } finally { @@ -1419,7 +1419,7 @@ public void RemoveAll(IEnumerable wrappers) { foreach (NativeObjectWrapper wrapper in wrappers) { - RemoveLocked(wrapper.ExternalComObject, wrapper); + Remove_Locked(wrapper.ExternalComObject, wrapper); } } finally @@ -1428,8 +1428,9 @@ public void RemoveAll(IEnumerable wrappers) } } - private void RemoveLocked(IntPtr comPointer, NativeObjectWrapper wrapper) + private void Remove_Locked(IntPtr comPointer, NativeObjectWrapper wrapper) { + Debug.Assert(_lock.IsWriteLockHeld); // This method is used in a scenario where we already have a lock on the cache, so we can skip acquiring the lock again. // TryGetOrCreateObjectForComInstanceInternal may have put a new entry into the cache // in the time between the GC cleared the contents of the GC handle but before the diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs index f2fa9986b041cf..eeefd7e9699bad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs @@ -87,7 +87,10 @@ internal static void ReleaseExternalObjectsFromCurrentThread() // passing them. nativeObjectWrapper.DisconnectTracker(); - wrappersToRemove.Add(nativeObjectWrapper); + if (nativeObjectWrapper.ComWrappers == GlobalInstanceForTrackerSupport) + { + wrappersToRemove.Add(nativeObjectWrapper); + } } } diff --git a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs index eaa93ee94fbd86..9b6772992afbe8 100644 --- a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs +++ b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs @@ -177,7 +177,10 @@ protected override void ReleaseObjects(IEnumerable objects) Assert.NotNull(o); } - throw new Exception() { HResult = ReleaseObjectsCallAck }; + if (ReturnInvalid) + { + throw new Exception() { HResult = ReleaseObjectsCallAck }; + } } private unsafe ComInterfaceEntry* ComputeVtablesForTestObject(Test obj, out int count) @@ -461,6 +464,26 @@ private static void ValidateNotifyEndOfReferenceTrackingOnThread() // Trigger the thread lifetime end API and verify the callback occurs. int hr = MockReferenceTrackerRuntime.Trigger_NotifyEndOfReferenceTrackingOnThread(); Assert.Equal(GlobalComWrappers.ReleaseObjectsCallAck, hr); + + // Validate that the RCW cache gets cleared when we call NotifyEndOfReferenceTrackingOnThread + GlobalComWrappers.Instance.ReturnInvalid = false; + IntPtr tracker = MockReferenceTrackerRuntime.CreateTrackerObject(); + + object rcw = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerSupport); + + // Make sure that we keep the tracker object alive even after we notify end of reference tracking on this thread. + Marshal.AddRef(tracker); + + const int S_OK = 0; + Assert.Equal(S_OK, MockReferenceTrackerRuntime.Trigger_NotifyEndOfReferenceTrackingOnThread()); + + // We should get a new RCW after we've released the reference tracked objects on this thread. + object rcwNew = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerSupport); + + // Release the extra ref we added above so we don't leak the object after the test. + Marshal.Release(tracker); + + Assert.NotSame(rcw, rcwNew); } } } From 36111ff5b23be48bf103667c1013e886da9c3a79 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 13:59:46 -0700 Subject: [PATCH 3/5] TrackerObject --- .../Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs index 9b6772992afbe8..569d4ac722f15c 100644 --- a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs +++ b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs @@ -469,7 +469,7 @@ private static void ValidateNotifyEndOfReferenceTrackingOnThread() GlobalComWrappers.Instance.ReturnInvalid = false; IntPtr tracker = MockReferenceTrackerRuntime.CreateTrackerObject(); - object rcw = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerSupport); + object rcw = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerObject); // Make sure that we keep the tracker object alive even after we notify end of reference tracking on this thread. Marshal.AddRef(tracker); @@ -478,7 +478,7 @@ private static void ValidateNotifyEndOfReferenceTrackingOnThread() Assert.Equal(S_OK, MockReferenceTrackerRuntime.Trigger_NotifyEndOfReferenceTrackingOnThread()); // We should get a new RCW after we've released the reference tracked objects on this thread. - object rcwNew = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerSupport); + object rcwNew = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerObject); // Release the extra ref we added above so we don't leak the object after the test. Marshal.Release(tracker); From 23f87b9cd07321d2b756bb66eceebd0b3229e81f Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 16:22:08 -0700 Subject: [PATCH 4/5] Update GlobalInstance.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../GlobalInstance/GlobalInstance.cs | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs index 569d4ac722f15c..810b8215915818 100644 --- a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs +++ b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs @@ -468,22 +468,30 @@ private static void ValidateNotifyEndOfReferenceTrackingOnThread() // Validate that the RCW cache gets cleared when we call NotifyEndOfReferenceTrackingOnThread GlobalComWrappers.Instance.ReturnInvalid = false; IntPtr tracker = MockReferenceTrackerRuntime.CreateTrackerObject(); + try + { + object rcw = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerObject); - object rcw = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerObject); - - // Make sure that we keep the tracker object alive even after we notify end of reference tracking on this thread. - Marshal.AddRef(tracker); - - const int S_OK = 0; - Assert.Equal(S_OK, MockReferenceTrackerRuntime.Trigger_NotifyEndOfReferenceTrackingOnThread()); + // Make sure that we keep the tracker object alive even after we notify end of reference tracking on this thread. + Marshal.AddRef(tracker); - // We should get a new RCW after we've released the reference tracked objects on this thread. - object rcwNew = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerObject); + const int S_OK = 0; + Assert.Equal(S_OK, MockReferenceTrackerRuntime.Trigger_NotifyEndOfReferenceTrackingOnThread()); - // Release the extra ref we added above so we don't leak the object after the test. - Marshal.Release(tracker); + // We should get a new RCW after we've released the reference tracked objects on this thread. + object rcwNew = GlobalComWrappers.Instance.GetOrCreateObjectForComInstance(tracker, CreateObjectFlags.TrackerObject); - Assert.NotSame(rcw, rcwNew); + Assert.NotSame(rcw, rcwNew); + } + finally + { + if (tracker != IntPtr.Zero) + { + // Release the extra ref we added above and the original ref from CreateTrackerObject. + Marshal.Release(tracker); + Marshal.Release(tracker); + } + } } } } From 721d9a04c6e1bc88d4f8ea3c976fe75ddb06ea21 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 16:24:27 -0700 Subject: [PATCH 5/5] Update TrackerObjectManager.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Runtime/InteropServices/TrackerObjectManager.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs index eeefd7e9699bad..85e3ee8bf96c65 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs @@ -93,13 +93,12 @@ internal static void ReleaseExternalObjectsFromCurrentThread() } } } - - // Remove the native object wrappers from the cache - // so we don't return released wrappers to the user if the native COM object - // happens to be reused. - GlobalInstanceForTrackerSupport.RemoveWrappersFromCache(wrappersToRemove); } + // Remove the native object wrappers from the cache + // so we don't return released wrappers to the user if the native COM object + // happens to be reused. + GlobalInstanceForTrackerSupport.RemoveWrappersFromCache(wrappersToRemove); GlobalInstanceForTrackerSupport.ReleaseObjects(objects); }