Skip to content

Commit

Permalink
Move ComWrappers AddRef to C/C++ (#110762)
Browse files Browse the repository at this point in the history
* 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

Co-authored-by: Aaron Robinson <[email protected]>
  • Loading branch information
jkotas and AaronRobinsonMSFT authored Dec 18, 2024
1 parent 5e52384 commit d7e85d9
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 10 deletions.
54 changes: 54 additions & 0 deletions src/coreclr/nativeaot/Runtime/HandleTableHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions src/coreclr/nativeaot/Runtime/unix/PalRedhawkInline.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/nativeaot/Runtime/windows/PalRedhawkInline.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,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;
}

Expand Down Expand Up @@ -1302,13 +1302,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)
{
Expand Down Expand Up @@ -1381,8 +1374,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,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")]
Expand Down

0 comments on commit d7e85d9

Please sign in to comment.