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] Don't wait for finalizers in 'IReferenceTrackerHost::ReleaseDisconnectedReferenceSources' #110558

Open
wants to merge 3 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 10, 2024

Backport of #110551 to release/9.0-staging

/cc @jkotas @Sergio0694

Customer Impact

  • Customer reported
  • Found internally

Intermittent hang when UWP/WinUI app is suspended. Reported by Windows Store.

Regression

  • Yes
  • No

This is .NET Native -> .NET 9 regression on UWP. The fix updates IReferenceTrackerHost::ReleaseDisconnectedReferenceSources implementation in .NET 9 to match .NET Native (ie reverts the offending change).

Testing

Fix validated on private build by Windows Store

Risk

Low

@jkotas
Copy link
Member

jkotas commented Dec 10, 2024

@Sergio0694 Would it be possible to validate the fix by building and testing Windows Store app with NAOT from main?

  • Wait a day for nightly build with the fix to get published
  • Add <add key="dotnet10" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet10/nuget/v3/index.json" /> to your nuget.config
  • Add <add key="dotnet10" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet10/nuget/v3/index.json" /> to your .csproj
  • Build & test

@Sergio0694
Copy link
Contributor

So I managed to get the Store to run on .NET 10 CoreCLR, but ILC is not happy 😅

image

Perhaps it would be simpler to just test with an SDK build from this branch? Is there a pipeline I can run to get it?

@MichalStrehovsky
Copy link
Member

Weird, this overload was added in #110234, this must be using some AotSdk with an old corelib (and likely old runtime too, without your fix). What are your repro steps?

If you download the latest .NET 10 SDK, it should already have the ILCompiler with your fix. So all that should be needed is to put it in the PATH, add NuGet.config with the .NET 10 feed, update Store to target .NET 10 and that's it.

@Sergio0694
Copy link
Contributor

That's exactly what I did 🥲

@Sergio0694
Copy link
Contributor

Update: I tested again (this time in Release, last time I was trying to run in Debug + Native AOT), and it works! 🎉

I can no longer repro the hangs (no spinning circle on the mouse cursor, no hangs in event viewer), and I can close and reopen the Store multiple times in rapid succession without getting stuck at the OS splash screen (due to the previous instance being hung)!

@jkotas
Copy link
Member

jkotas commented Dec 14, 2024

@Sergio0694 Thank you for testing the fix! I have submitted the change into servicing approval process.

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. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Dec 14, 2024
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Dec 14, 2024
@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 6, 2025
@jeffschwMSFT jeffschwMSFT modified the milestones: 9.0.x, 9.0.2 Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Interop-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants