[release/9.0-staging] Move ComWrappers AddRef to C/C++ (#110815)

* Move ComWrappers AddRef to C/C++

Xaml invokes AddRef while holding a lock that it *also* holds while a GC is in progress. Managed AddRef had to synchronize with the GC that caused intermittent deadlocks with the other thread holding Xaml's lock.

This change reverts the managed AddRef implementation to match .NET Native and CoreCLR.

Fixes #110747

* Apply suggestions from code review

Co-authored-by: Aaron Robinson <[email protected]>

* Update src/coreclr/nativeaot/Runtime/HandleTableHelpers.cpp

* Update src/coreclr/nativeaot/Runtime/HandleTableHelpers.cpp

* Build break

* Update src/coreclr/nativeaot/Runtime/HandleTableHelpers.cpp

* Apply suggestions from code review

* Update src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs

---------

Co-authored-by: Jan Kotas <[email protected]>
Co-authored-by: Aaron Robinson <[email protected]>
Co-authored-by: Jeff Schwartz <[email protected]>

Author: 4 people

src/coreclr/nativeaot/Runtime/HandleTableHelpers.cpp

@@ -70,3 +70,57 @@ FCIMPL2(void, RhUnregisterRefCountedHandleCallback, void * pCallout, MethodTable
     RestrictedCallouts::UnregisterRefCountedHandleCallback(pCallout, pTypeFilter);
 }
 FCIMPLEND
+
+// This structure mirrors the managed type System.Runtime.InteropServices.ComWrappers.ManagedObjectWrapper.
+struct ManagedObjectWrapper
+{
+    intptr_t HolderHandle;
+    uint64_t RefCount;
+
+    int32_t UserDefinedCount;
+    void* /* ComInterfaceEntry */ UserDefined;
+    void* /* InternalComInterfaceDispatch* */ Dispatches;
+
+    int32_t /* CreateComInterfaceFlagsEx */ Flags;
+
+    uint32_t AddRef()
+    {
+        return GetComCount((uint64_t)PalInterlockedIncrement64((int64_t*)&RefCount));
+    }
+
+    static const uint64_t ComRefCountMask = 0x000000007fffffffUL;
+
+    static uint32_t GetComCount(uint64_t c)
+    {
+        return (uint32_t)(c & ComRefCountMask);
+    }
+};
+
+// This structure mirrors the managed type System.Runtime.InteropServices.ComWrappers.InternalComInterfaceDispatch.
+struct InternalComInterfaceDispatch
+{
+    void* Vtable;
+    ManagedObjectWrapper* _thisPtr;
+};
+
+static ManagedObjectWrapper* ToManagedObjectWrapper(void* dispatchPtr)
+{
+    return ((InternalComInterfaceDispatch*)dispatchPtr)->_thisPtr;
+}
+
+//
+// AddRef is implemented in native code so that it does not need to synchronize with the GC. This is important because Xaml
+// invokes AddRef while holding a lock that it *also* holds while a GC is in progress.  If AddRef was managed, we would have
+// to synchronize with the GC before entering AddRef, which would deadlock with the other thread holding Xaml's lock.
+//
+static uint32_t __stdcall IUnknown_AddRef(void* pComThis)
+{
+    ManagedObjectWrapper* wrapper = ToManagedObjectWrapper(pComThis);
+    return wrapper->AddRef();
+}
+
+FCIMPL0(void*, RhGetIUnknownAddRef)
+{
+    return (void*)&IUnknown_AddRef;
+}
+FCIMPLEND

src/coreclr/nativeaot/Runtime/unix/PalRedhawkInline.h

@@ -30,6 +30,13 @@ FORCEINLINE int32_t PalInterlockedIncrement(_Inout_ int32_t volatile *pDst)
     return result;
 }
 
+FORCEINLINE int64_t PalInterlockedIncrement64(_Inout_ int64_t volatile *pDst)
+{
+    int64_t result = __sync_add_and_fetch(pDst, 1);
+    PalInterlockedOperationBarrier();
+    return result;
+}
+
 FORCEINLINE int32_t PalInterlockedDecrement(_Inout_ int32_t volatile *pDst)
 {
     int32_t result = __sync_sub_and_fetch(pDst, 1);

src/coreclr/nativeaot/Runtime/windows/PalRedhawkInline.h

@@ -67,13 +67,31 @@ FORCEINLINE int64_t PalInterlockedExchange64(_Inout_ int64_t volatile *pDst, int
                                           iOldValue) != iOldValue);
     return iOldValue;
 }
+
+FORCEINLINE int64_t PalInterlockedIncrement64(_Inout_ int64_t volatile *Addend)
+{
+    int64_t Old;
+    do {
+        Old = *Addend;
+    } while (PalInterlockedCompareExchange64(Addend,
+                                          Old + 1,
+                                          Old) != Old);
+    return Old + 1;
+}
 #else // HOST_X86
 EXTERN_C int64_t _InterlockedExchange64(int64_t volatile *, int64_t);
 #pragma intrinsic(_InterlockedExchange64)
 FORCEINLINE int64_t PalInterlockedExchange64(_Inout_ int64_t volatile *pDst, int64_t iValue)
 {
     return _InterlockedExchange64(pDst, iValue);
 }
+
+EXTERN_C int64_t _InterlockedIncrement64(int64_t volatile *);
+#pragma intrinsic(_InterlockedIncrement64)
+FORCEINLINE int64_t PalInterlockedIncrement64(_Inout_ int64_t volatile *pDst)
+{
+    return _InterlockedIncrement64(pDst);
+}
 #endif // HOST_X86
 
 #if defined(HOST_AMD64) || defined(HOST_ARM64)

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

@@ -1250,7 +1250,7 @@ public static void RegisterForMarshalling(ComWrappers instance)
         public static unsafe void GetIUnknownImpl(out IntPtr fpQueryInterface, out IntPtr fpAddRef, out IntPtr fpRelease)
         {
             fpQueryInterface = (IntPtr)(delegate* unmanaged<IntPtr, Guid*, IntPtr*, int>)&ComWrappers.IUnknown_QueryInterface;
-            fpAddRef = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IUnknown_AddRef;
+            fpAddRef = RuntimeImports.RhGetIUnknownAddRef(); // Implemented in C/C++ to avoid GC transitions
             fpRelease = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IUnknown_Release;
         }
 
@@ -1395,13 +1395,6 @@ internal static unsafe int IUnknown_QueryInterface(IntPtr pThis, Guid* guid, Int
             return wrapper->QueryInterface(in *guid, out *ppObject);
         }
 
-        [UnmanagedCallersOnly]
-        internal static unsafe uint IUnknown_AddRef(IntPtr pThis)
-        {
-            ManagedObjectWrapper* wrapper = ComInterfaceDispatch.ToManagedObjectWrapper((ComInterfaceDispatch*)pThis);
-            return wrapper->AddRef();
-        }
-
         [UnmanagedCallersOnly]
         internal static unsafe uint IUnknown_Release(IntPtr pThis)
         {
@@ -1474,8 +1467,7 @@ private static unsafe IntPtr CreateDefaultIReferenceTrackerTargetVftbl()
         {
             IntPtr* vftbl = (IntPtr*)RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(ComWrappers), 7 * sizeof(IntPtr));
             vftbl[0] = (IntPtr)(delegate* unmanaged<IntPtr, Guid*, IntPtr*, int>)&ComWrappers.IReferenceTrackerTarget_QueryInterface;
-            vftbl[1] = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IUnknown_AddRef;
-            vftbl[2] = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IUnknown_Release;
+            GetIUnknownImpl(out _, out vftbl[1], out vftbl[2]);
             vftbl[3] = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IReferenceTrackerTarget_AddRefFromReferenceTracker;
             vftbl[4] = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IReferenceTrackerTarget_ReleaseFromReferenceTracker;
             vftbl[5] = (IntPtr)(delegate* unmanaged<IntPtr, uint>)&ComWrappers.IReferenceTrackerTarget_Peg;

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs

@@ -511,6 +511,10 @@ internal enum GcRestrictedCalloutKind
         [RuntimeImport(RuntimeLibrary, "RhUnregisterRefCountedHandleCallback")]
         internal static extern unsafe void RhUnregisterRefCountedHandleCallback(IntPtr pCalloutMethod, MethodTable* pTypeFilter);
 
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        [RuntimeImport(RuntimeLibrary, "RhGetIUnknownAddRef")]
+        internal static extern IntPtr RhGetIUnknownAddRef();
+
 #if FEATURE_OBJCMARSHAL
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         [RuntimeImport(RuntimeLibrary, "RhRegisterObjectiveCMarshalBeginEndCallback")]