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

Fix incorrect nativeaot event thread / sequence number on shutdown #88941

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jul 15, 2023

On shutdown of the event pipe infrastructure, any active session is disabled and the process info event is written (and rundown - but nativeaot doesn't emit any). The shutdown was being done in the atexit handler. We use a thread_local EventPipeAotThreadHolderTLS to track information about that thread as it pertains to event pipe - for example, the sequence number for events from that thread. That gets destructed, such that when we get to writing out the process info event during process exit, we write the event with the incorrect information (a reset sequence number). As a result, analysis (via TraceEvent, for example) flags that all the events before it have been dropped.

This switches to using our existing thread exit callbacks to handle event pipe thread info cleanup instead of relying on the destructor and merges the paths around event pipe thread holder management for windows/non-windows.

@ghost
Copy link

ghost commented Jul 15, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

On shutdown of the event pipe infrastructure, any active session is disabled and the process info event is written (and rundown - but nativeaot doesn't emit any). The shutdown was being done in the atexit handler. We use a thread_local EventPipeAotThreadHolderTLS to track information about that thread as it pertains to event pipe - for example, the sequence number for events from that thread. That gets destructed, such that when we get to writing out the process info event during process exit, we write the event with the incorrect information (a reset sequence number). As a result, analysis (via TraceEvent, for example) flags that all the events before it have been dropped.

This separates the shutdown of the event pipe infrastructure into an explicit call rather than part of process exit.

Author: elinor-fung
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@jkotas
Copy link
Member

jkotas commented Jul 15, 2023

That gets destructed,

Would it be better to not destruct it by keeping this info in a thread static that does not have a destructor?

Re-introducing RhShutdown that we got rid of in #80063 is replacing one set of problems with a different set of problems.

@elinor-fung elinor-fung merged commit d672bcc into dotnet:main Jul 20, 2023
@elinor-fung elinor-fung deleted the threadSequenceNumber branch July 20, 2023 13:44
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2023
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