Skip to content

Improve shutdown sequence of DebugAdapter #110

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

Open
daviwil opened this issue Jan 10, 2016 · 5 comments
Open

Improve shutdown sequence of DebugAdapter #110

daviwil opened this issue Jan 10, 2016 · 5 comments
Labels
Issue-Bug A bug to squash.

Comments

@daviwil
Copy link
Contributor

daviwil commented Jan 10, 2016

There is a timing issue with the shutdown sequence of the DebugAdapter which occasionally causes test hangs because the terminated event is not received. My guess is that this event never gets fully sent because of the timing of the shutdown sequence relative to some event handlers that get fired at that time. Compare the following two log files, the invocation state changes are interesting.

GoodRun.log.txt
BadRun.log.txt

There are some additional notes in the BadRun.log.txt file at the end.

It's likely that the shutdown sequence will need to be rethought so that the order of events is predictable and all messages get sent before the process terminates.

@daviwil daviwil added the Issue-Bug A bug to squash. label Jan 10, 2016
@daviwil daviwil added this to the Backlog milestone Jan 10, 2016
@rkeithhill
Copy link
Contributor

BTW in StdioClientChannel.cs I wonder if we should change wrap this call:

this.serviceProcess.Kill();

in a try/catch. I've observed this line throwing at times when running the unit tests (usually when I've screwed up something) because the service has already exited and then we try to kill it.

@daviwil
Copy link
Contributor Author

daviwil commented Feb 28, 2016

Definitely, good point. I'll take that into account when I work on this. I've got some stuff I've already started to improve the async model so that CancellationTokens and failures flow through more easily (also with a fix for the CPU spin in MesaageReader). Will try to get that in for 0.5.0.

@rkeithhill
Copy link
Contributor

I also notice this issue when I'm manually stepping through a unit test. By the time I get to the line that sends the DisconnectRequest, the process has already terminated. That results in an unhandled exception and causes the unit test to fail. These tests rarely (if ever) fail this way when when not run under the debugger but we gotta be able to debug unit tests, right. :-)

@daviwil
Copy link
Contributor Author

daviwil commented Mar 1, 2016

Yeah, it needs to work correctly for sure. I keep seeing an annoying ThreadAbortException when running the tests in the debugger, is that what you're seeing too?

@rkeithhill
Copy link
Contributor

I don't think that was what I was seeing but I was "experimenting" and likely causing the host to exit prematurely. :-)

TylerLeonhardt pushed a commit to TylerLeonhardt/PowerShellEditorServices that referenced this issue Feb 26, 2019
@andyleejordan andyleejordan removed this from the Backlog milestone Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug A bug to squash.
Projects
None yet
Development

No branches or pull requests

3 participants