Skip to content

Don't wait for finalizers in 'IReferenceTrackerHost::ReleaseDisconnectedReferenceSources' - #110551

Merged
jkotas merged 3 commits into
dotnet:mainfrom
Sergio0694:user/sergiopedri/tracker-host-finalizer-wait
Dec 10, 2024
Merged

Don't wait for finalizers in 'IReferenceTrackerHost::ReleaseDisconnectedReferenceSources'#110551
jkotas merged 3 commits into
dotnet:mainfrom
Sergio0694:user/sergiopedri/tracker-host-finalizer-wait

Conversation

@Sergio0694

Copy link
Copy Markdown
Contributor

Contributes to #109538

This PR updates the IReferenceTrackerHost::ReleaseDisconnectedReferenceSources implementation for CoreCLR and NativeAOT to match what .NET Native was doing, and not wait for finalizers, to avoid deadlocks in ASTA scenarios (UWP). Finalizers will just continue running normally, and if the process is suspended (which can only happen on UWP anyway), they'll resume when the process is resumed later on. Worst case scenario this would only cause a non-optimal memory use cleanup before suspension (which is better than a deadlock anyway). Can be revisited and improved for .NET 10 if needed.

@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Dec 9, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@Sergio0694

Copy link
Copy Markdown
ContributorAuthor

@jkotas I'm targeting main and can then cherry-pick into release/9.0, is this the right workflow for this repo or should I be targeting the release branch first and then cherry-pick from there into main later? I can do whichever is preferred 🙂

@jkotas

Copy link
Copy Markdown
Member

I'm targeting main and can then cherry-pick into release/9.0, is this the right workflow for this repo

Yes, the right workflow for this repo is to merge the change into main first.

@jkoritzinskyjkoritzinsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a comment wording nit, otherwise LGTM

Comment threadsrc/coreclr/interop/trackerobjectmanager.cpp Outdated
@jkotas

Copy link
Copy Markdown
Member

/backport to release/9.0-staging

@github-actions

Copy link
Copy Markdown
Contributor

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

hez2010 pushed a commit to hez2010/runtime that referenced this pull request Dec 14, 2024
…tedReferenceSources' (dotnet#110551)
This PR updates the IReferenceTrackerHost::ReleaseDisconnectedReferenceSources implementation for CoreCLR and NativeAOT to match what .NET Native was doing, and not wait for finalizers, to avoid deadlocks in ASTA scenarios (UWP). Finalizers will just continue running normally, and if the process is suspended (which can only happen on UWP anyway), they'll resume when the process is resumed later on. Worst case scenario this would only cause a non-optimal memory use cleanup before suspension (which is better than a deadlock anyway). Can be revisited and improved for .NET 10 if needed.
Contributes to dotnet#109538
Sergio0694 added a commit to Sergio0694/runtime that referenced this pull request Jan 7, 2025
…tedReferenceSources' (dotnet#110551)
This PR updates the IReferenceTrackerHost::ReleaseDisconnectedReferenceSources implementation for CoreCLR and NativeAOT to match what .NET Native was doing, and not wait for finalizers, to avoid deadlocks in ASTA scenarios (UWP). Finalizers will just continue running normally, and if the process is suspended (which can only happen on UWP anyway), they'll resume when the process is resumed later on. Worst case scenario this would only cause a non-optimal memory use cleanup before suspension (which is better than a deadlock anyway). Can be revisited and improved for .NET 10 if needed.
Contributes to dotnet#109538
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 9, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Interop-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Sergio0694@jkotas@jkoritzinsky@AaronRobinsonMSFT