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

VSCode "restart extensions" on any extension update makes vscode-powershell start a new terminal #4986

Open
1 of 6 tasks
o-l-a-v opened this issue May 13, 2024 · 5 comments
Open
1 of 6 tasks
Assignees

Comments

@o-l-a-v
Copy link

o-l-a-v commented May 13, 2024

Prerequisites

  • I have written a descriptive issue title.
  • I have searched all open and closed issues to ensure it has not already been reported.
  • I have read the troubleshooting guide.
  • I am sure this issue is with the extension itself and does not reproduce in a standalone PowerShell instance.
  • I have verified that I am using the latest version of Visual Studio Code and the PowerShell extension.
  • If this is a security issue, I have read the security issue reporting guidance.

Summary

In VSCode, if updating any VSCode extension, then push the "restart extensions" button, vscode-powershell fires up a new terminal without killing the running one.

240513-vscode-restart-extensions.mp4

PowerShell Version

Name                           Value
----                           -----
PSVersion                      7.4.2
PSEdition                      Core
GitCommitId                    7.4.2
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0


Name             : Visual Studio Code Host
Version          : 2024.2.1
InstanceId       : a740115b-1966-49cf-bfba-db38a7044e50
UI               : System.Management.Automation.Internal.Host.InternalHostUserInterface
CurrentCulture   : en-SE
CurrentUICulture : en-US
PrivateData      : Microsoft.PowerShell.ConsoleHost+ConsoleColorProxy
DebuggerEnabled  : True
IsRunspacePushed : False
Runspace         : System.Management.Automation.Runspaces.LocalRunspace

Visual Studio Code Version

1.89.1
dc96b837cf6bb4af9cd736aa3af08cf8279f7685
x64

Extension Version

ms-vscode.powershell@2024.2.1

Steps to Reproduce

  • I guess settings.json "powershell.startAutomatically": true could be relevant here?
  • Make sure vscode-powershell terminal is running.
  • Downgrade any extension. "Restart extensions" will fire up a new vscode-powershell terminal

Visuals

image

Logs

No response

@o-l-a-v o-l-a-v added Issue-Bug A bug to squash. Needs: Triage Maintainer attention needed! labels May 13, 2024
@JustinGrote
Copy link
Collaborator

JustinGrote commented May 14, 2024

Thanks for the report. @andyleejordan being able to restart the extension host without restarting all of vscode is a new feature, and I'm not exactly sure how the cleanup/teardown works but I'm guessing we need to add a teardown step for the extension or probably a disposable that makes sure to clean up the extension terminal.

EDIT: Also FWIW I could reproduce even when not upgrading, just running restart extension host
image

@JustinGrote JustinGrote added Area-Extension Terminal and removed Needs: Triage Maintainer attention needed! labels May 14, 2024
@andyleejordan
Copy link
Member

Yeah, we have logic setup to handle tear down...but those handlers actually need to fire. This is the main one:

// Subscribe a log event for when the terminal closes (this fires for
// all terminals and the event itself checks if it's our terminal). This
// subscription should happen before we create the terminal so if it
// fails immediately, the event fires.
this.consoleCloseSubscription = vscode.window.onDidCloseTerminal((terminal) => { this.onTerminalClose(terminal); });

If the extension host is being restarted, I assume that means our handler can't fire, which would result in this behavior. We'll have to look into if there are any like "deactivate" hooks to implement.

@JustinGrote
Copy link
Collaborator

JustinGrote commented May 14, 2024

@andyleejordan you probably need to register onTerminalClose as a disposable to the context.subscriptions of the extension context array. Alternatively we can just implement a deactivate method at the root level.
https://code.visualstudio.com/api/get-started/extension-anatomy#extension-entry-file

@andyleejordan
Copy link
Member

andyleejordan commented May 15, 2024

Sooo as far as I can tell all those things are wired up correctly. I think disposing a Terminal object from within the extension doesn't actually kill that terminal (which is very weird but I'm watching dipsose() get called and the terminal just sitting there alive and well). VS Code handles kililng them all on reloadWindow but it's specifically not killing them on restartExtensionHost. I'm seeing if @Tyriar has any idea how we can manually kill a terminal on deactivation of the extension.

@andyleejordan andyleejordan self-assigned this May 16, 2024
@Tyriar
Copy link
Contributor

Tyriar commented May 28, 2024

I think what's probably happening here is dispose is being called but it's a sync call and the request may not be able to complete as it requires a round trip to the renderer process:

Renderer:

https://github.com/microsoft/vscode/blob/c6e45e96a6b0fe94e0dae5b13ab4167d69ec9788/src/vs/workbench/api/common/extHostTerminalService.ts#L584-L591

Calls ext host:

https://github.com/microsoft/vscode/blob/c6e45e96a6b0fe94e0dae5b13ab4167d69ec9788/src/vs/workbench/api/common/extHostTerminalService.ts#L584-L591

@andyleejordan could you dispose the terminal/do the clean up after the restart happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants