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

Root the System.Runtime EventSource #108266

Merged
merged 3 commits into from
Sep 27, 2024
Merged

Conversation

noahfalk
Copy link
Member

The System.Runtime EventSource (RuntimeEventSource), was unintentionally being garbage collected because it wasn't rooted. This caused runtime EventCounters to no longer be available after GC occurred.

This was a regression from a recent change (#106014). That change accidentally converted the static field that was intended to the root the object into a property getter that returned a new instance each time it was called. This fix converts the property back into a statically initialized field.

This will fix #107919 once it is backported.

The System.Runtime EventSource (RuntimeEventSource), was unintentionally being garbage collected because it wasn't rooted. This caused runtime EventCounters to no longer be available after GC occurred.

This was a regression from a recent change (dotnet#106014). That change accidentally converted the static field that was intended to the root the object into a property getter that returned a new instance each time it was called. This fix converts the property back into a statically initialized field.

This will fix dotnet#107919 once it is backported.
@noahfalk
Copy link
Member Author

@davmason @brianrob PTAL

@noahfalk
Copy link
Member Author

cc @dotnet/dotnet-diag

@noahfalk
Copy link
Member Author

Thanks for the review, all feedback was applied!

@noahfalk noahfalk merged commit 69de6dd into dotnet:main Sep 27, 2024
148 checks passed
@tarekgh
Copy link
Member

tarekgh commented Sep 27, 2024

@noahfalk will you port this to 9.0 release?

@noahfalk
Copy link
Member Author

/backport to release/9.0

Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11077946461

@noahfalk
Copy link
Member Author

@noahfalk will you port this to 9.0 release?

Yep, it was on my todo list, just didn't get a chance to do it yesterday.

sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
Root the System.Runtime EventSource

The System.Runtime EventSource (RuntimeEventSource), was unintentionally being garbage collected because it wasn't rooted. This caused runtime EventCounters to no longer be available after GC occurred.

This was a regression from a recent change (dotnet#106014). That change accidentally converted the static field that was intended to the root the object into a property getter that returned a new instance each time it was called. This fix converts the property back to being initialized only once.

This will fix dotnet#107919 once it is backported.

Co-authored-by: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Oct 28, 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.

.NET event counters from System.Runtime are not reported in some circumstances in .NET 9 RC 1
6 participants