-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Exit the lock before we call into user code and handle losing the race for the RCW table #110828
base: main
Are you sure you want to change the base?
Conversation
…e for the RCW table.
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
1. Register that we've created a managed object as a wrapper for a COM object first (equivalent to ExtObjCxtCache in CoreCLR) 2. Register the NativeObjectWrapper for lifetime management (equivalent to the sync-block interop info in CoreCLR). This ensures that we have the same behavior as CoreCLR.
Tests for this specific deadlock are still TODO, but I've reorganized the code for the NativeAOT ComWrappers implementation to match the same order of logic as the CoreCLR implementation and encapsulated each step into separate helper functions (which also helped significantly clarify lock usage). |
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding all the comments!! 😄
Left a few more notes.
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
Outdated
Show resolved
Hide resolved
… the "RCW address -> wrapper" cache. Use a WeakReference as we don't want to keep the wrapper alive any longer with it being in the cache.
…here's only ever one NativeObjectWrapper we try to register, the one that's in the RCW cache
Fixes #110823
Still needs tests