Skip to content

Conversation

@sywhang
Copy link
Contributor

@sywhang sywhang commented Sep 20, 2019

@josalem just reported an issue where if the app exits before dotnet-counters, we get a timeout on StopTracing command and it crashes dotnet-counters with an unhandled exception. This fixes that bad behavior.

@sywhang sywhang requested a review from josalem September 20, 2019 22:52
@sywhang
Copy link
Contributor Author

sywhang commented Sep 20, 2019

@mikem8361 Is release/3.0 branch still open? I'd like to merge this into 3.0 official release possible

Copy link
Contributor

@josalem josalem left a comment

Choose a reason for hiding this comment

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

Is there a way we can differentiate between "The pipe never existed" and "the pipe has stopped existing"? Right now I think this will catch for both scenarios, which means we won't have a -1 exit code for pointing counters at an incompatible process. For now, I think this is fine though.

@sywhang
Copy link
Contributor Author

sywhang commented Sep 20, 2019

@josalem thanks for catching that. That's a really good point. I can move the exception handling to StopMonitor to distinguish the two.

@mikem8361
Copy link
Contributor

mikem8361 commented Sep 20, 2019 via email

@sywhang
Copy link
Contributor Author

sywhang commented Sep 20, 2019

@mikem8361 Do you have a preference between merging this to master first, then cherry-picking it, or just creating a separate PR to release/3.0?

@mikem8361
Copy link
Contributor

mikem8361 commented Sep 20, 2019 via email

@sywhang
Copy link
Contributor Author

sywhang commented Sep 20, 2019

OK as soon as the CI is green I'll merge this to master and cherry-pick it to release/3.0. Thanks Mike!

@sywhang sywhang merged commit ec26ac1 into dotnet:master Sep 21, 2019
sywhang added a commit to sywhang/diagnostics that referenced this pull request Sep 21, 2019
* Fix a timeout exception causing unhandled exception crash

* Fix the same issue on Unix too

* Move the exception handling to StopMonitor to distinguish actually incompatible runtimes from early exits

* fix build error
sywhang added a commit that referenced this pull request Sep 21, 2019
* Fix a timeout exception causing unhandled exception crash

* Fix the same issue on Unix too

* Move the exception handling to StopMonitor to distinguish actually incompatible runtimes from early exits

* fix build error
@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 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