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

Restarting the pty-host (because it was unresponsive) doesn't reconnect terminals #117548

Closed
eamodio opened this issue Feb 24, 2021 · 3 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue terminal Integrated terminal issues verified Verification succeeded
Milestone

Comments

@eamodio
Copy link
Contributor

eamodio commented Feb 24, 2021

Testing #117265 on Windows

Steps:

  1. Open 2 terminals
  2. Start a lot of output ls -r in one
  3. Kill the terminal outputting lots of data
  4. The other terminal is unresponsive
  5. Restart the pty-host when prompted
  6. Expect that either the unresponsive terminal is reconnected (which I'm assuming is not possible), or it would just be closed.

It seems odd to leave it open in a "disconnected" state -- as it makes it seem like it can be re-connected.

Also on a couple of occasions, the terminal didn't get (disconnected) in the name, though most of the time it did.

@Tyriar
Copy link
Member

Tyriar commented Feb 24, 2021

I thought it was best to leave it in the disconnected state in case it had important data in it. Do you think we should instead write disconnected + relaunching and relaunch without clearing it maybe?

@pvogel1967
Copy link

Hitting restart Pty Host has the current effect of doing nothing useful so yeah, I think you should self repair here, ideally without me having to click a dialog, but if I have to click, fine. Don't leave me with a hung terminal even if I take the recommended action.

@Tyriar Tyriar added terminal Integrated terminal issues polish Cleanup and polish issue labels Mar 12, 2021
@Tyriar Tyriar added this to the March 2021 milestone Mar 12, 2021
@Tyriar Tyriar added the bug Issue identified by VS Code Team member as probable bug label Mar 19, 2021
@Tyriar
Copy link
Member

Tyriar commented Mar 19, 2021

Thanks for the suggestions, here's the new experience:

recording (13)

Also no notification shows up anymore for when the pty host shuts down, only when it's unresponsive which happens when the proc hangs or restart failed several times. The reason for this is because it's possible to recover to unresponsiveness. Hopefully #71966 will be fixed soon so it very rarely happens and we're left with a really resilient pty host.

@Tyriar Tyriar closed this as completed in f514652 Mar 19, 2021
@RMacfarlane RMacfarlane added the verified Verification succeeded label Mar 26, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue terminal Integrated terminal issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants
@eamodio @pvogel1967 @Tyriar @RMacfarlane and others