Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes hang in WinUI apps published to AOT #104583

Merged
merged 8 commits into from
Sep 12, 2024
Merged

Fixes hang in WinUI apps published to AOT #104583

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0b9bf74
Fix exception during GC callback in WinUI scenarios due to removing f…
manodasanW Jul 7, 2024
95ef91b
Move common logic
manodasanW Jul 7, 2024
fbd03f4
Change collection type to HashSet to match with JIT implementation an…
manodasanW Jul 9, 2024
0cedbd0
Move to using a custom collection to protect against GC freezing thre…
manodasanW Jul 19, 2024
4839e79
Address PR feedback by using alternative approach to handle race
manodasanW Jul 20, 2024
cb82992
Address PR feedback
manodasanW Jul 26, 2024
d4a49c8
Address PR feedback around handle being freed.
manodasanW Sep 11, 2024
c8d6e2d
Merge branch 'main' into manodasanw/fixComWrappersAotHang
manodasanW Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 34 additions & 22 deletions ...iveaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public abstract partial class ComWrappers
private static readonly Guid IID_IWeakReferenceSource = new Guid(0x00000038, 0, 0, 0xC0, 0, 0, 0, 0, 0, 0, 0x46);

private static readonly ConditionalWeakTable<object, NativeObjectWrapper> s_rcwTable = new ConditionalWeakTable<object, NativeObjectWrapper>();
private static readonly List<GCHandle> s_referenceTrackerNativeObjectWrapperCache = new List<GCHandle>();
private static readonly HashSet<GCHandle> s_referenceTrackerNativeObjectWrapperCache = new HashSet<GCHandle>();
private static readonly Lock s_nativeObjectWrapperCacheLock = new Lock(useTrivialWaits: true);

private readonly ConditionalWeakTable<object, ManagedObjectWrapperHolder> _ccwTable = new ConditionalWeakTable<object, ManagedObjectWrapperHolder>();
private readonly Lock _lock = new Lock(useTrivialWaits: true);
Expand Down Expand Up @@ -639,7 +640,10 @@ public override void Release()
// Remove the entry from the cache that keeps track of the active NativeObjectWrappers.
if (_nativeObjectWrapperWeakHandle.IsAllocated)
{
s_referenceTrackerNativeObjectWrapperCache.Remove(_nativeObjectWrapperWeakHandle);
using (s_nativeObjectWrapperCacheLock.EnterScope())
{
s_referenceTrackerNativeObjectWrapperCache.Remove(_nativeObjectWrapperWeakHandle);
}
_nativeObjectWrapperWeakHandle.Free();
}

Expand Down Expand Up @@ -986,10 +990,7 @@ private unsafe bool TryGetOrCreateObjectForComInstanceInternal(
throw new NotSupportedException();
}
_rcwCache.Add(identity, wrapper._proxyHandle);
if (wrapper is ReferenceTrackerNativeObjectWrapper referenceTrackerNativeObjectWrapper)
{
s_referenceTrackerNativeObjectWrapperCache.Add(referenceTrackerNativeObjectWrapper._nativeObjectWrapperWeakHandle);
}
AddWrapperToReferenceTrackerHandleCache(wrapper);
return true;
}
}
Expand Down Expand Up @@ -1042,10 +1043,7 @@ private unsafe bool TryGetOrCreateObjectForComInstanceInternal(
wrapper.Release();
throw new NotSupportedException();
}
if (wrapper is ReferenceTrackerNativeObjectWrapper referenceTrackerNativeObjectWrapper)
{
s_referenceTrackerNativeObjectWrapperCache.Add(referenceTrackerNativeObjectWrapper._nativeObjectWrapperWeakHandle);
}
AddWrapperToReferenceTrackerHandleCache(wrapper);
return true;
}

Expand Down Expand Up @@ -1081,17 +1079,25 @@ private unsafe bool TryGetOrCreateObjectForComInstanceInternal(
throw new NotSupportedException();
}
_rcwCache.Add(identity, wrapper._proxyHandle);
if (wrapper is ReferenceTrackerNativeObjectWrapper referenceTrackerNativeObjectWrapper)
{
s_referenceTrackerNativeObjectWrapperCache.Add(referenceTrackerNativeObjectWrapper._nativeObjectWrapperWeakHandle);
}
AddWrapperToReferenceTrackerHandleCache(wrapper);
}
}

return true;
}
#pragma warning restore IDE0060

private static void AddWrapperToReferenceTrackerHandleCache(NativeObjectWrapper wrapper)
{
if (wrapper is ReferenceTrackerNativeObjectWrapper referenceTrackerNativeObjectWrapper)
{
using (s_nativeObjectWrapperCacheLock.EnterScope())
{
s_referenceTrackerNativeObjectWrapperCache.Add(referenceTrackerNativeObjectWrapper._nativeObjectWrapperWeakHandle);
}
}
}

private void RemoveRCWFromCache(IntPtr comPointer, GCHandle expectedValue)
{
using (_lock.EnterScope())
Expand Down Expand Up @@ -1222,17 +1228,21 @@ internal static void ReleaseExternalObjectsFromCurrentThread()
IntPtr contextToken = GetContextToken();

List<object> objects = new List<object>();
foreach (GCHandle weakNativeObjectWrapperHandle in s_referenceTrackerNativeObjectWrapperCache)

using (s_nativeObjectWrapperCacheLock.EnterScope())
{
ReferenceTrackerNativeObjectWrapper? nativeObjectWrapper = Unsafe.As<ReferenceTrackerNativeObjectWrapper?>(weakNativeObjectWrapperHandle.Target);
if (nativeObjectWrapper != null &&
nativeObjectWrapper._contextToken == contextToken)
foreach (GCHandle weakNativeObjectWrapperHandle in s_referenceTrackerNativeObjectWrapperCache)
{
objects.Add(nativeObjectWrapper._proxyHandle.Target);
ReferenceTrackerNativeObjectWrapper? nativeObjectWrapper = Unsafe.As<ReferenceTrackerNativeObjectWrapper?>(weakNativeObjectWrapperHandle.Target);
if (nativeObjectWrapper != null &&
nativeObjectWrapper._contextToken == contextToken)
{
objects.Add(nativeObjectWrapper._proxyHandle.Target);

// Separate the wrapper from the tracker runtime prior to
// passing them.
nativeObjectWrapper.DisconnectTracker();
// Separate the wrapper from the tracker runtime prior to
// passing them.
nativeObjectWrapper.DisconnectTracker();
}
}
}

Expand All @@ -1244,6 +1254,7 @@ internal static unsafe void WalkExternalTrackerObjects()
{
bool walkFailed = false;

// No lock needed to access s_referenceTrackerNativeObjectWrapperCache as this is during GC callback.
foreach (GCHandle weakNativeObjectWrapperHandle in s_referenceTrackerNativeObjectWrapperCache)
{
ReferenceTrackerNativeObjectWrapper? nativeObjectWrapper = Unsafe.As<ReferenceTrackerNativeObjectWrapper?>(weakNativeObjectWrapperHandle.Target);
Expand Down Expand Up @@ -1272,6 +1283,7 @@ internal static unsafe void WalkExternalTrackerObjects()
// Used during GC callback
internal static void DetachNonPromotedObjects()
{
// No lock needed to access s_referenceTrackerNativeObjectWrapperCache as this is during GC callback.
foreach (GCHandle weakNativeObjectWrapperHandle in s_referenceTrackerNativeObjectWrapperCache)
{
ReferenceTrackerNativeObjectWrapper? nativeObjectWrapper = Unsafe.As<ReferenceTrackerNativeObjectWrapper?>(weakNativeObjectWrapperHandle.Target);
Expand Down
Loading