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

o-l-a-v commented 1 month ago

Prerequisites

[X] 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.

https://github.com/PowerShell/vscode-powershell/assets/6450056/7d366ac4-b6a2-401f-a032-b4caeadfb3a5

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

Logs

No response

JustinGrote commented 1 month ago

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

andyleejordan commented 1 month ago

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

https://github.com/PowerShell/vscode-powershell/blob/009341541fd6a91f02d88dff804b8a67761e308b/src/process.ts#L112-L116

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 commented 1 month ago

@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 commented 1 month ago

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.

Tyriar commented 1 month ago

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?

PowerShell / vscode-powershell