Reduce the GC handle cost of every RCW in ComWrappers

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

@@ -449,6 +449,15 @@ internal static unsafe bool ObjectHasComponentSize(object obj)
             return GetMethodTable(obj)->HasComponentSize;
         }
 
+        // Returns true iff the type of the object requires finalization,
+        // which includes a finalizer inherited from a base type.
+        internal static unsafe bool ObjectHasFinalizer(object obj)
+        {
+            bool hasFinalizer = GetMethodTable(obj)->HasFinalizer;
+            GC.KeepAlive(obj); // Keep MethodTable alive
+            return hasFinalizer;
+        }
+
         /// <summary>
         /// Boxes a given value using an input <see cref="MethodTable"/> to determine its type.
         /// </summary>

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs

@@ -213,6 +213,13 @@ internal static unsafe bool ObjectHasComponentSize(object obj)
             return GetMethodTable(obj)->HasComponentSize;
         }
 
+        // Returns true iff the type of the object requires finalization,
+        // which includes a finalizer inherited from a base type.
+        internal static unsafe bool ObjectHasFinalizer(object obj)
+        {
+            return GetMethodTable(obj)->IsFinalizable;
+        }
+
         public static void PrepareMethod(RuntimeMethodHandle method)
         {
             if (method.Value == IntPtr.Zero)

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.NativeAot.cs

@@ -186,7 +186,8 @@ internal static void DetachNonPromotedObjects()
                 ReferenceTrackerNativeObjectWrapper? nativeObjectWrapper = Unsafe.As<ReferenceTrackerNativeObjectWrapper>(weakNativeObjectWrapperHandle.Target);
                 if (nativeObjectWrapper != null &&
                     nativeObjectWrapper.TrackerObject != IntPtr.Zero &&
-                    !RuntimeImports.RhIsPromoted(nativeObjectWrapper.ProxyHandle.Target))
+                    nativeObjectWrapper.ProxyHandle.TryGetTarget(out object? proxyTarget) &&
+                    !RuntimeImports.RhIsPromoted(proxyTarget))
                 {
                     // Notify the wrapper it was not promoted and is being collected.
                     BeforeWrapperFinalized(nativeObjectWrapper.TrackerObject);
@@ -205,9 +206,9 @@ internal static unsafe class FindReferenceTargetsCallback
         internal ref struct Instance
         {
             private readonly IntPtr _vtable; // First field is IUnknown based vtable.
-            public GCHandle RootObject;
+            public WeakGCHandle<object> RootObject;
 
-            public Instance(GCHandle handle)
+            public Instance(WeakGCHandle<object> handle)
             {
                 _vtable = (IntPtr)Unsafe.AsPointer(in FindReferenceTargetsCallback.Vftbl);
                 RootObject = handle;
@@ -240,7 +241,11 @@ private static unsafe int IFindReferenceTargetsCallback_FoundTrackerTarget(IntPt
                 return HResults.E_POINTER;
             }
 
-            object sourceObject = ((FindReferenceTargetsCallback.Instance*)pThis)->RootObject.Target!;
+            _ = ((FindReferenceTargetsCallback.Instance*)pThis)->RootObject.TryGetTarget(out object? sourceObject);
+
+            // The callback is only ever set up with the handle of an RCW that was alive at the time, and
+            // that RCW keeps its wrapper alive, so the handle is expected to still have its target here
+            Debug.Assert(sourceObject is not null);
 
             if (!TryGetObject(referenceTrackerTarget, out object? targetObject))
             {
@@ -253,7 +258,7 @@ private static unsafe int IFindReferenceTargetsCallback_FoundTrackerTarget(IntPt
             }
 
             // Notify the runtime a reference path was found.
-            return TrackerObjectManager.AddReferencePath(sourceObject, targetObject) ? HResults.S_OK : HResults.S_FALSE;
+            return TrackerObjectManager.AddReferencePath(sourceObject!, targetObject) ? HResults.S_OK : HResults.S_FALSE;
         }
 
         internal struct ReferenceTargetsVftbl

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs

@@ -548,8 +548,8 @@ internal unsafe class NativeObjectWrapper
             private ComWrappers _comWrappers;
             private IntPtr _externalComObject;
             private IntPtr _inner;
-            private GCHandle _proxyHandle;
-            private GCHandle _proxyHandleTrackingResurrection;
+            private WeakGCHandle<object> _proxyHandle;
+            private WeakGCHandle<object> _proxyHandleTrackingResurrection;
             private readonly bool _aggregatedManagedObjectWrapper;
             private readonly bool _uniqueInstance;
 
@@ -594,14 +594,31 @@ protected NativeObjectWrapper(IntPtr externalComObject, IntPtr inner, ComWrapper
                 _inner = inner;
                 _comWrappers = comWrappers;
                 _uniqueInstance = flags.HasFlag(CreateObjectFlags.UniqueInstance);
-                _proxyHandle = GCHandle.Alloc(comProxy, GCHandleType.Weak);
 
-                // We have a separate handle tracking resurrection as we want to make sure
-                // we clean up the NativeObjectWrapper only after the RCW has been finalized
-                // due to it can access the native object in the finalizer. At the same time,
-                // we want other callers which are using ProxyHandle such as the reference tracker runtime
-                // to see the object as not alive once it is eligible for finalization.
-                _proxyHandleTrackingResurrection = GCHandle.Alloc(comProxy, GCHandleType.WeakTrackResurrection);
+                // The wrapper's finalizer must not release anything while the RCW is still able to observe the
+                // native object, which is why a handle that tracks resurrection is needed: unlike a plain weak
+                // handle, it stays set until the RCW has actually been collected, rather than merely becoming
+                // unreachable. Callers such as the reference tracker runtime want the opposite, and need to see
+                // the RCW as gone as soon as it is eligible for finalization, which is what 'ProxyHandle' is for.
+                //
+                // Those two only disagree while the RCW is unreachable but not yet collected. An RCW that has
+                // no finalizer is never in that state on its own account, so a single handle can serve both
+                // purposes, halving the handles every such RCW costs. It can still be put in that state by
+                // something else's finalizer holding on to it, and then resurrecting it, and in that case having
+                // the one handle track resurrection is what keeps this wrapper from tearing down state the
+                // resurrected RCW still needs. Reporting such an RCW as alive is also the honest answer, as it
+                // may well be about to become reachable again.
+                //
+                // An RCW that does have a finalizer, whether its own or an inherited one, does reach that state
+                // on its own, and there the two meanings genuinely differ, so it pays for both handles.
+                bool proxyHasFinalizer = RuntimeHelpers.ObjectHasFinalizer(comProxy);
+
+                _proxyHandle = new WeakGCHandle<object>(comProxy, trackResurrection: !proxyHasFinalizer);
+
+                if (proxyHasFinalizer)
+                {
+                    _proxyHandleTrackingResurrection = new WeakGCHandle<object>(comProxy, trackResurrection: true);
+                }
 
                 // If this is an aggregation scenario and the identity object
                 // is a managed object wrapper, we need to call Release() to
@@ -617,7 +634,7 @@ protected NativeObjectWrapper(IntPtr externalComObject, IntPtr inner, ComWrapper
 
             internal IntPtr ExternalComObject => _externalComObject;
             internal ComWrappers ComWrappers => _comWrappers;
-            internal GCHandle ProxyHandle => _proxyHandle;
+            internal WeakGCHandle<object> ProxyHandle => _proxyHandle;
             internal bool IsUniqueInstance => _uniqueInstance;
             internal bool IsAggregatedWithManagedObjectWrapper => _aggregatedManagedObjectWrapper;
 
@@ -629,15 +646,8 @@ public virtual void Release()
                     _comWrappers = null!;
                 }
 
-                if (_proxyHandle.IsAllocated)
-                {
-                    _proxyHandle.Free();
-                }
-
-                if (_proxyHandleTrackingResurrection.IsAllocated)
-                {
-                    _proxyHandleTrackingResurrection.Free();
-                }
+                _proxyHandle.Dispose();
+                _proxyHandleTrackingResurrection.Dispose();
 
                 // If the inner was supplied, we need to release our reference.
                 if (_inner != IntPtr.Zero)
@@ -651,7 +661,15 @@ public virtual void Release()
 
             ~NativeObjectWrapper()
             {
-                if (_proxyHandleTrackingResurrection.IsAllocated && _proxyHandleTrackingResurrection.Target != null)
+                // When the RCW has no finalizer, no second handle was allocated and the proxy handle is
+                // the one tracking resurrection, so it answers this question just as well. Neither is allocated
+                // once this wrapper has been released, which happens eagerly when one loses a registration race,
+                // and then there is nothing left to keep alive for.
+                WeakGCHandle<object> resurrectionHandle = _proxyHandleTrackingResurrection.IsAllocated
+                    ? _proxyHandleTrackingResurrection
+                    : _proxyHandle;
+
+                if (resurrectionHandle.IsAllocated && resurrectionHandle.TryGetTarget(out _))
                 {
                     // The RCW object has not been fully collected, so it still
                     // can make calls on the native object in its finalizer.
@@ -1261,7 +1279,7 @@ private void RegisterWrapperForObject(NativeObjectWrapper wrapper, object comPro
             // for the same COM instance, but in that case we'll be passed the same NativeObjectWrapper instance
             // for both threads. In that case, it doesn't matter which thread adds the entry to the NativeObjectWrapper table
             // as the entry is always the same pair.
-            Debug.Assert(wrapper.ProxyHandle.Target == comProxy);
+            Debug.Assert(wrapper.ProxyHandle.TryGetTarget(out object? proxyTarget) && proxyTarget == comProxy);
             Debug.Assert(wrapper.IsUniqueInstance || _rcwCache.FindProxyForComInstance(wrapper.ExternalComObject) == comProxy);
 
             // Add the input wrapper bound to the COM proxy, if there isn't one already. If another thread raced
@@ -1431,7 +1449,7 @@ public Bucket()
                     _lock.EnterWriteLock();
                     try
                     {
-                        Debug.Assert(wrapper.ProxyHandle.Target == comProxy);
+                        Debug.Assert(wrapper.ProxyHandle.TryGetTarget(out object? proxyTarget) && proxyTarget == comProxy);
                         ref WeakGCHandle<NativeObjectWrapper> rcwEntry = ref CollectionsMarshal.GetValueRefOrAddDefault(_cache, comPointer, out bool exists);
                         if (!exists)
                         {
@@ -1447,10 +1465,9 @@ public Bucket()
                         }
                         else
                         {
-                            object? existingProxy = cachedWrapper.ProxyHandle.Target;
                             // The target NativeObjectWrapper was not collected, but we need to make sure
                             // that the proxy object is still alive.
-                            if (existingProxy is not null)
+                            if (cachedWrapper.ProxyHandle.TryGetTarget(out object? existingProxy))
                             {
                                 // The existing proxy object is still alive, we will use that.
                                 return (cachedWrapper, existingProxy);
@@ -1482,7 +1499,7 @@ public Bucket()
                             return null;
                         }
                         if (existingHandle.TryGetTarget(out NativeObjectWrapper? cachedWrapper)
-                            && cachedWrapper.ProxyHandle.Target is object cachedProxy)
+                            && cachedWrapper.ProxyHandle.TryGetTarget(out object? cachedProxy))
                         {
                             // The target exists and is still alive. Return it.
                             return cachedProxy;

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.cs

@@ -86,8 +86,7 @@ internal static void ReleaseExternalObjectsFromCurrentThread()
                         {
                             wrappersToRemove.Add(nativeObjectWrapper);
 
-                            object? target = nativeObjectWrapper.ProxyHandle.Target;
-                            if (target != null)
+                            if (nativeObjectWrapper.ProxyHandle.TryGetTarget(out object? target))
                             {
                                 objects.Add(target);
                             }

src/tests/Interop/COM/ComWrappers/API/Program.cs

@@ -33,6 +33,8 @@ static TestComWrappers()
                 fpWrappedQueryInterface = MockReferenceTrackerRuntime.WrapQueryInterface(fpQueryInterface);
             }
 
+            public bool UseManualReleaseITestObjectWrapper { get; init; }
+
             protected unsafe override ComInterfaceEntry* ComputeVtables(object obj, CreateComInterfaceFlags flags, out int count)
             {
                 ComInterfaceEntry* entryRaw = null;
@@ -133,7 +135,14 @@ protected override object CreateObject(IntPtr externalComObject, CreateObjectFla
                 hr = Marshal.QueryInterface(externalComObject, typeof(ITest).GUID, out iTest);
                 if (hr == 0)
                 {
-                    return new ITestObjectWrapper(iTest);
+                    if (UseManualReleaseITestObjectWrapper)
+                    {
+                        return new ManualReleaseITestObjectWrapper(iTest);
+                    }
+                    else
+                    {
+                        return new ITestObjectWrapper(iTest);
+                    }
                 }
 
                 Assert.Fail("The COM object should support ITrackerObject or ITest for all tests in this test suite.");
@@ -418,9 +427,9 @@ unsafe static void CallSetValue(TestComWrappers wrappers, Test testInstance, int
         [MethodImpl(MethodImplOptions.NoInlining)]
         [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
         [Fact]
-        public void ValidateResurrection()
+        public void ValidateManagedObjectWrapperResurrection()
         {
-            Console.WriteLine($"Running {nameof(ValidateResurrection)}...");
+            Console.WriteLine($"Running {nameof(ValidateManagedObjectWrapperResurrection)}...");
 
             var wrappers = new TestComWrappers();
 
@@ -621,6 +630,72 @@ public void ValidateMappingAPIs()
             Marshal.Release(unmanagedObjIUnknown);
         }
 
+        class Resurrecter()
+        {
+            public ManualReleaseITestObjectWrapper? UnmanagedWrapper;
+
+            ~Resurrecter()
+            {
+                if (UnmanagedWrapper != null)
+                {
+                    GC.ReRegisterForFinalize(this);
+                }
+            }
+        }
+
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+        [Fact]
+        public void ValidateNativeObjectWrapperResurrection()
+        {
+            Console.WriteLine($"Running {nameof(ValidateNativeObjectWrapperResurrection)}...");
+
+            var cw = new TestComWrappers()
+            {
+                UseManualReleaseITestObjectWrapper = true,
+            };
+
+            WeakGCHandle<Resurrecter> resurrecter;
+            nint unmanagedObj = AllocateWrapper(cw, out resurrecter);
+            Assert.Equal(0, Marshal.QueryInterface(unmanagedObj, IUnknownVtbl.IID_IUnknown, out IntPtr unmanagedObjIUnknown));
+            ForceGC();
+            AssertNativeObjectWrapperAlive(cw, resurrecter, unmanagedObjIUnknown);
+
+            resurrecter.Dispose();
+            Marshal.Release(unmanagedObjIUnknown);
+            Assert.Equal(0, Marshal.Release(unmanagedObj));
+
+            [MethodImpl(MethodImplOptions.NoInlining)]
+            static nint AllocateWrapper(ComWrappers cw, out WeakGCHandle<Resurrecter> handle)
+            {
+                Test test = new();
+                nint comWrapper = cw.GetOrCreateComInterfaceForObject(test, CreateComInterfaceFlags.None);
+                Assert.NotEqual(IntPtr.Zero, comWrapper);
+
+                var unmanagedWrapper = (ManualReleaseITestObjectWrapper)cw.GetOrCreateObjectForComInstance(comWrapper, CreateObjectFlags.UniqueInstance);
+                Resurrecter resurrecter = new()
+                {
+                    UnmanagedWrapper = unmanagedWrapper,
+                };
+                handle = new WeakGCHandle<Resurrecter>(resurrecter, true);
+                return comWrapper;
+            }
+
+            [MethodImpl(MethodImplOptions.NoInlining)]
+            static void AssertNativeObjectWrapperAlive(ComWrappers cw, WeakGCHandle<Resurrecter> handle, IntPtr unmanagedObj)
+            {
+                Assert.True(handle.TryGetTarget(out Resurrecter resurrecter));
+                ManualReleaseITestObjectWrapper? unmanagedWrapper = resurrecter.UnmanagedWrapper;
+                Assert.NotNull(resurrecter);
+                Assert.True(ComWrappers.TryGetComInstance(unmanagedWrapper, out IntPtr unmanagedObjOther));
+                Assert.Equal(unmanagedObj, unmanagedObjOther);
+                resurrecter.UnmanagedWrapper = null;
+                Marshal.Release(unmanagedObjOther);
+                unmanagedWrapper.FinalRelease();
+            }
+        }
+
         [MethodImpl(MethodImplOptions.NoInlining)]
         [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
         [Fact]

src/tests/Interop/COM/ComWrappers/Common.cs

@@ -94,13 +94,13 @@ public static int SetValueInternal(IntPtr dispatchPtr, int i)
         }
     }
 
-    public class ITestObjectWrapper : ITest
+    public class ITestObjectWrapperBase : ITest
     {
         private readonly ITestVtbl._SetValue _setValue;
-        private readonly IntPtr _ptr;
-        private bool _released;
+        protected readonly IntPtr _ptr;
+        protected bool _released;
 
-        public ITestObjectWrapper(IntPtr ptr)
+        public ITestObjectWrapperBase(IntPtr ptr)
         {
             _ptr = ptr;
             VtblPtr inst = Marshal.PtrToStructure<VtblPtr>(ptr);
@@ -117,15 +117,31 @@ public int FinalRelease()
             return count;
         }
 
+        public void SetValue(int i) => _setValue(_ptr, i);
+    }
+
+    public class ManualReleaseITestObjectWrapper : ITestObjectWrapperBase
+    {
+        public ManualReleaseITestObjectWrapper(IntPtr ptr)
+            : base(ptr)
+        {
+        }
+    }
+
+    public class ITestObjectWrapper : ITestObjectWrapperBase
+    {
+        public ITestObjectWrapper(IntPtr ptr)
+            : base(ptr)
+        {
+        }
+
         ~ITestObjectWrapper()
         {
             if (_ptr != IntPtr.Zero && !_released)
             {
                 Marshal.Release(_ptr);
             }
         }
-
-        public void SetValue(int i) => _setValue(_ptr, i);
     }
 
     //