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

EventPipe: Don't clean up thread list on shutdown #96936

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

davmason
Copy link
Member

We had a report of a crash in EventPipe where the app was shutting down, and after _ep_threads_lock was freed a new thread was trying to fire an event and ran in to an AV trying to lock _ep_threads_lock to check if the thread was known.

This PR cleans up some dead code and makes _ep_threads_lock follow the approach we've been taking for some time now, to not clean up and let the OS deal with it after process exit.

Copy link
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

I think we should revisit the shutdown/uninitialize approach of EventPipe at some time, so we can split what's being called at shutdown vs potential unloading of the library in embedding scenarios in cases where the process continue to execute after the runtime has been unloaded and potentially reloaded.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

👍

@davmason
Copy link
Member Author

Build analysis says all failures are known, merging.

@davmason davmason merged commit dbb335c into dotnet:main Jan 17, 2024
157 of 160 checks passed
@davmason
Copy link
Member Author

/backport to release/8.0-staging

Copy link
Contributor

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

tmds pushed a commit to tmds/runtime that referenced this pull request Jan 23, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants