Skip to content

Fix bad context state after debugging #1210

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

Merged
merged 6 commits into from
Feb 28, 2020

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Feb 27, 2020

Fixes PowerShell/vscode-powershell#2364.
Fixes PowerShell/vscode-powershell#2488.
May fix PowerShell/vscode-powershell#2396.

After debugging is stopped, the PowerShellContextService was being marked as being in session state aborted. Now we set the state to ready.

This fix isn't ideal, since it's made without a full understanding of the conceptual state machine of the PowerShellContextService, but it does fix the issues for the coming release while we sift through the actual architectural cause of the problem (and ideally fix it down the line)

@SeeminglyScience
Copy link
Collaborator

Can you test CTRL + C in various situations? Like hit a debugger stop, manually run Start-Sleep 10, try to CTRL + C?

I feel like I did something like this early on and it broke something but I can't remember what or why...

@rjmholt
Copy link
Contributor Author

rjmholt commented Feb 28, 2020

Like hit a debugger stop, manually run Start-Sleep 10, try to CTRL + C?

This one seems to work nicely.

I imagine there is an issue somewhere here. But if we don't find it, I think this change should go in, since the devil we know is quite bad

@rjmholt
Copy link
Contributor Author

rjmholt commented Feb 28, 2020

Ah, it's the second Ctrl-C that gets ignored. I'm ok with that one for now. If we can find a better implementation before Tuesday, we can put it in this PR. Otherwise will bugfix it later

@rjmholt
Copy link
Contributor Author

rjmholt commented Feb 28, 2020

@SeeminglyScience do you know where we process ctrl C? I'd like to get a better picture of what's going on so I can think about what a fix might look like

@rjmholt
Copy link
Contributor Author

rjmholt commented Feb 28, 2020

Ok, I've now made it so that sleep 10 works. I'm not sure if this could corrupt some state, but it seems not to have in my limited uses.

Basically abort opportunistically tries to acquire the lock to fire the execution status changed event. If there's a command running, it won't get the lock and no event is fired.

@PowerShell PowerShell deleted a comment from rjmholt Feb 28, 2020
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

As long as ctrl c isn't broken I'm cool with this as a temp fix. Maybe we should get an issue opened to take a pass at this for vnextnext.

@rjmholt rjmholt merged commit 35fd46d into PowerShell:master Feb 28, 2020
@rjmholt rjmholt deleted the abort-ready-fix branch February 28, 2020 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants