Uh oh!
There was an error while loading. Please reload this page.
Move ComWrappers AddRef to C/C++ - #110762
Conversation
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. Fixesdotnet#110747
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Sergio0694
left a comment
There was a problem hiding this comment.
Thank you!! 😄
Tested with the Microsoft Store and could not repro any hangs:
- Checked out
release/9.0-staging(51fd1e2) - Cherry-pick commits from #110558
- Cherry-pick commits from this PR
- Build with
.\build.cmd clr.aot -c Release - Set
IlcSdkPathin the Store .csproj toruntime\artifacts\bin\coreclr\windows.x64.Release\aotsdk\ - Set
GenerateAppxPackageOnBuild - Switch to Release
- Deploy
I then tried to use the Store for a while, and also did the same thing that triggered the hang last time (opening and closing it in quick succession a couple dozen times). I could not repro the issue and everything seemed to be working great 🎉
Uh oh!
There was an error while loading. Please reload this page.
| #if false // Implemented in C/C++ to avoid GC transitions | ||
| [UnmanagedCallersOnly] | ||
| internal static unsafe uint IUnknown_AddRef(IntPtr pThis) | ||
| { | ||
| ManagedObjectWrapper* wrapper = ComInterfaceDispatch.ToManagedObjectWrapper((ComInterfaceDispatch*)pThis); | ||
| return wrapper->AddRef(); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
I don't really like the #if false pattern. I'd rather we just include a comment on line 1160 explaining that we have it implemented in C/C++ to avoid GC transitions.
AaronRobinsonMSFT
left a comment
There was a problem hiding this comment.
I agree with @jkoritzinsky's comment about the #if false style. Other than that, LGTM.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…e/InteropServices/ComWrappers.NativeAot.cs
jkotas
commented
Dec 18, 2024
/backport to release/9.0-staging |
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/12396026201 |
* 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. Fixesdotnet#110747 Co-authored-by: Aaron Robinson <arobins@microsoft.com>
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