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.

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

[release/9.0-staging] Exit the lock before we call into user code and handle losing the race for the RCW table #111162

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 7, 2025

Backport of #110828 to release/9.0-staging

/cc @jkoritzinsky

Customer Impact

  • Customer reported
  • Found internally

Microsoft Store app deadlock when running on NativeAOT, hit when registering a COM object in ComWrappers across apartments.

Regression

  • Yes
  • No

Regression from CoreCLR, not a regression in NativeAOT

Testing

Added test coverage for cross-thread and cross-apartment scenarios. Verified with a private build for the Microsoft Store.

Risk

Low. We've validated that the tests covering this case pass, all regression tests pass, and the Microsoft Store team has validated the fix. In addition, the change in logic now has the NativeAOT ComWrappers implementation matching the CoreCLR ComWrappers implementation, which gives us more confidence in the NativeAOT ComWrappers implementation.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

Sorry, something went wrong.

jkoritzinsky and others added 16 commits January 7, 2025 15:47
…e for the RCW table.
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.
… 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.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…here's only ever one NativeObjectWrapper we try to register, the one that's in the RCW cache

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Sergio Pedri <sergio0694@live.com>
Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. please get a code review. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jan 7, 2025
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Jan 7, 2025
@jkoritzinsky jkoritzinsky added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 7, 2025
@jkoritzinsky jkoritzinsky merged commit 08b542e into release/9.0-staging Jan 8, 2025
96 of 99 checks passed
@jkoritzinsky jkoritzinsky deleted the backport/pr-110828-to-release/9.0-staging branch January 8, 2025 00:44
@jeffschwMSFT jeffschwMSFT modified the milestones: 9.0.x, 9.0.2 Jan 8, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Feb 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Interop-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants