From e46e0d438db9914251dfc22a56dc41b6ec54fdde Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 18 Mar 2026 15:49:10 -0700 Subject: [PATCH 1/9] 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 9df46043178eda..4b2b16c68bf4eb 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 @@ -1287,6 +1287,11 @@ private static void AddWrapperToReferenceTrackerHandleCache(NativeObjectWrapper } } + internal void RemoveWrappersFromCache(IEnumerable wrappers) + { + _rcwCache.RemoveAll(wrappers); + } + private sealed class RcwCache { private readonly Lock _lock = new Lock(useTrivialWaits: true); @@ -1363,20 +1368,42 @@ public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) { lock (_lock) { - // 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); } } } + + 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 df0053629c9ca6..73b287c1f93e50 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 6415f93af2a174fb4527c68921f4071e2b50dca0 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 13:00:25 -0700 Subject: [PATCH 2/9] 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 4b2b16c68bf4eb..d8e3c017fe7f73 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 @@ -1368,7 +1368,7 @@ public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) { lock (_lock) { - RemoveLocked(comPointer, wrapper); + Remove_Locked(comPointer, wrapper); } finally { @@ -1383,13 +1383,14 @@ public void RemoveAll(IEnumerable wrappers) { foreach (NativeObjectWrapper wrapper in wrappers) { - RemoveLocked(wrapper.ExternalComObject, wrapper); + Remove_Locked(wrapper.ExternalComObject, wrapper); } } } - 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 73b287c1f93e50..2871972a32d142 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 b6aed237d20a398965343c060989bfe29bb3dc7b Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 13:59:46 -0700 Subject: [PATCH 3/9] 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 c25e62c1459e03e534672a09d3ef122c4ab99320 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 16:22:08 -0700 Subject: [PATCH 4/9] 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 ba155b06299d850c39440637ce3aeba76071bd92 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 19 Mar 2026 16:24:27 -0700 Subject: [PATCH 5/9] 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 2871972a32d142..2af1dd85b68e08 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); } From b6d116526b7069f02cb092d78ea6018d633f3d22 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 20 Mar 2026 10:42:22 -0700 Subject: [PATCH 6/9] Move object collection for ReleaseObjects to only be for the global tracker instance --- .../InteropServices/TrackerObjectManager.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 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 2af1dd85b68e08..6ebac14be3c2a2 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 @@ -77,19 +77,24 @@ internal static void ReleaseExternalObjectsFromCurrentThread() if (nativeObjectWrapper != null && nativeObjectWrapper._contextToken == contextToken) { - object? target = nativeObjectWrapper.ProxyHandle.Target; - if (target != null) - { - objects.Add(target); - } - // Separate the wrapper from the tracker runtime prior to // passing them. nativeObjectWrapper.DisconnectTracker(); + // If this object is associated with the global instance for tracker support, + // then we can request that instance to clear out the native object wrapper's state + // to ensure the object gets released now. + // Also, we will remove the wrappers from the cache to ensure a stale wrapper + // isn't returned in the future. if (nativeObjectWrapper.ComWrappers == GlobalInstanceForTrackerSupport) { wrappersToRemove.Add(nativeObjectWrapper); + + object? target = nativeObjectWrapper.ProxyHandle.Target; + if (target != null) + { + objects.Add(target); + } } } } From 65ad18b66252a8dcb1167a6df5117f7c52b2e0aa Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 20 Mar 2026 10:47:48 -0700 Subject: [PATCH 7/9] Reorder DisconnectTracker call in TrackerObjectManager --- .../Runtime/InteropServices/TrackerObjectManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 6ebac14be3c2a2..385ff438a3d44d 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 @@ -77,10 +77,6 @@ internal static void ReleaseExternalObjectsFromCurrentThread() if (nativeObjectWrapper != null && nativeObjectWrapper._contextToken == contextToken) { - // Separate the wrapper from the tracker runtime prior to - // passing them. - nativeObjectWrapper.DisconnectTracker(); - // If this object is associated with the global instance for tracker support, // then we can request that instance to clear out the native object wrapper's state // to ensure the object gets released now. @@ -96,6 +92,10 @@ internal static void ReleaseExternalObjectsFromCurrentThread() objects.Add(target); } } + + // Separate the wrapper from the tracker runtime prior to + // passing them. + nativeObjectWrapper.DisconnectTracker(); } } } From fb71051a88830ea529c8bdae9cf0e4270f02f5ff Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 20 Mar 2026 11:06:01 -0700 Subject: [PATCH 8/9] Fix bad backport (again) --- .../src/System/Runtime/InteropServices/ComWrappers.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 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 d8e3c017fe7f73..5a4d281c2f328e 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 @@ -1366,9 +1366,12 @@ private sealed class RcwCache public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) { - lock (_lock) + try { - Remove_Locked(comPointer, wrapper); + lock (_lock) + { + Remove_Locked(comPointer, wrapper); + } } finally { @@ -1386,6 +1389,10 @@ public void RemoveAll(IEnumerable wrappers) Remove_Locked(wrapper.ExternalComObject, wrapper); } } + finally + { + _lock.ExitWriteLock(); + } } private void Remove_Locked(IntPtr comPointer, NativeObjectWrapper wrapper) From 274d7c2224a3a99ebffdef305d24b437e1729842 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 20 Mar 2026 11:07:27 -0700 Subject: [PATCH 9/9] We use a different locking mechanism in .NET 10 --- .../Runtime/InteropServices/ComWrappers.cs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 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 5a4d281c2f328e..d4571f02fa8b66 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 @@ -1366,38 +1366,25 @@ private sealed class RcwCache public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper) { - try - { - lock (_lock) - { - Remove_Locked(comPointer, wrapper); - } - } - finally + lock (_lock) { - _lock.ExitWriteLock(); + Remove_Locked(comPointer, wrapper); } } public void RemoveAll(IEnumerable wrappers) { - _lock.EnterWriteLock(); - try + lock (_lock) { foreach (NativeObjectWrapper wrapper in wrappers) { Remove_Locked(wrapper.ExternalComObject, wrapper); } } - finally - { - _lock.ExitWriteLock(); - } } 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