dotnet / diagnostics

This repository contains the source code for various .NET Core runtime diagnostic tools and documents.
MIT License
1.19k stars 354 forks source link

Improve cancellation handling for dotnet-stack #4996

Closed noahfalk closed 1 month ago

noahfalk commented 1 month ago

Fixes https://github.com/dotnet/diagnostics/issues/4826

When running dotnet-stack against an unresponsive target process, there were various points where dotnet-stack wouldn't correctly cancel when Ctrl-C was pressed. There were several underlying issues:

The change in behavior for EventPipeSession.Dispose() is strictly speaking a breaking change, although callers would need to emply some dubious code patterns to observe the difference. The most likely way code could observe the difference is if thread 1 is reading from the EventStream at the same time thread 2 called Dispose(). Previously this would have caused thread 1 to start receiving rundown events although it was also a race condition between thread 1 reading from the stream and thread 2 disposing the stream. Its possible some tool could have worked successfully if thread 1 always won the race in practice. If any code was doing that pattern then now thread 1 will observe the stream is disposed without seeing the rundown events first. The proper way to ensure seeing all the rundown events would be to explicitly call EventPipeSession.Stop(), then read all the remaining data and reach the end of stream marker, then Dispose() the session.

I looked through all the usage of EventPipeSession in our existing tools and it looked like all of them were already using Stop() properly.

noahfalk commented 1 month ago

@dotnet/dotnet-diag PTAL FYI @dotnet/dotnet-monitor - this PR includes a behavioral change for EventPipeSession.Dispose().