Fixes hang in WinUI apps published to AOT

[manodasanW]

In the AOT version of ComWrappers, we keep track of the native object wrappers that implement IReferenceTracker and iterate on them during GC callbacks as part of the reference walk. We were running into scenarios where GCHandles in that collection were already freed but are still in the collection and thereby causing unexpected exceptions. This looks to be the result of a race that is happening when we are removing from that collection and adding to it as the removal has no locking at all around it and is failing randomly. The adding to the collection does have some locking, but that locking is for another cache and isn't consistent everywhere we add to it. This change adds a lock which is used during interactions with that collection. The lock is not used during the GC callback scenarios as the threads would be frozen already and we can end up in a scenario where the lock is already being held.

Given we are now doing locking during removal in the finalizer, I am also updating the collection type to a HashSet rather than a List. This makes it consistent with what we do in the JIT version of this implementation and also looking at some brief traces of WinUIGallery in both scenarios, there is definitely an improvement seen during removal with the HashSet approach while not much additional cost when adding it or iterating over it.

Fixes #104582

[hez2010]

Related: microsoft/microsoft-ui-xaml#9928
I think this need to be fixed before WASDK 1.6 GA.

[Sergio0694]

"I think this need to be fixed before WASDK 1.6 GA."

This seems infeasible at this point. WASDK 1.6 will go GA shortly, and this fix will likely only make it to .NET 9 GA at the earliest.

[FrayxRulez]

This seems infeasible at this point. WASDK 1.6 will go GA shortly, and this fix will likely only make it to .NET 9 GA at the earliest.

What’s a reasonable ETA in months? So that I can drop WinUI for now and go back to UWP, while planning for the next steps.

[manodasanW]

@FrayxRulez I don't have an ETA I can state yet as I still need to have a conversation with the .NET folks. But I do have plans on getting the PR feedback addressed very soon so we can then figure that out.

[Scottj1s]

Is there any conceivable workaround for this, while awaiting a .NET update?

[hez2010]

Is there any conceivable workaround for this

No. Unfortunately it's a show stopper.

[jkotas]

LGTM

[agocke]

/backport to release/9.0

[github-actions]

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10836541436

[manodasanW]

/backport to release/8.0-staging

[github-actions]

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/10837018960

[github-actions]

@manodasanW an error occurred while backporting to release/8.0-staging, please check the run log for details!

Error: @manodasanW is not a repo collaborator, backporting is not allowed. If you're a collaborator please make sure your dotnet team membership visibility is set to Public on https://github.com/orgs/dotnet/people?query=manodasanW

[jkoritzinsky]

/backport to release/8.0-staging

[github-actions]

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/10838290100

[github-actions]

@jkoritzinsky backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix exception during GC callback in WinUI scenarios due to removing from and adding to the nativeObjectReference list around the same time and causing removal to fail.
Using index info to reconstruct a base tree...
M	src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Applying: Move common logic
Using index info to reconstruct a base tree...
M	src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Applying: Change collection type to HashSet to match with JIT implementation and to improve performance of remove given it now takes a lock
Applying: Move to using a custom collection to protect against GC freezing threads during add / remove.
error: sha1 information is lacking or useless (src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0004 Move to using a custom collection to protect against GC freezing threads during add / remove.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[github-actions]

@jkoritzinsky an error occurred while backporting to release/8.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

[FrayxRulez]

Is this fix already available in either .NET 9 or .NET 8?

[filipnavara]

Is this fix already available in either .NET 9 or .NET 8?

No. It will be available in .NET 9 RC2.