diff --git a/docs/cspell.json b/docs/cspell.json index 73aa77b8167be..4af871bd2506a 100644 --- a/docs/cspell.json +++ b/docs/cspell.json @@ -953,6 +953,7 @@ "winadj", "windowsaccountname", "windowsdesktop", + "winpty", "winscp", "winserver", "workgroups", diff --git a/docs/pages/connect-your-client/teleport-connect.mdx b/docs/pages/connect-your-client/teleport-connect.mdx index 30c974cf86495..91739ecb26aa4 100644 --- a/docs/pages/connect-your-client/teleport-connect.mdx +++ b/docs/pages/connect-your-client/teleport-connect.mdx @@ -446,6 +446,7 @@ Below is the list of the supported config properties. | `theme` | `system` | Color theme for the app. Available modes: `light`, `dark`, `system`. | | `terminal.fontFamily` | `Menlo, Monaco, monospace` on macOS
`Consolas, monospace` on Windows
`'Droid Sans Mono', monospace` on Linux | Font family for the terminal. | | `terminal.fontSize` | 15 | Font size for the terminal. | +| `terminal.windowsBackend` | `auto` | `auto` uses modern [ConPTY](https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-console-conpty/) system if available, which requires Windows 10 (19H1) or above. Set to `winpty` to use winpty even if ConPTY is available. | | `usageReporting.enabled` | `false` | Enables collecting anonymous usage data (see [Telemetry](#telemetry)). | | `keymap.tab1` - `keymap.tab9` | `Command+1` - `Command+9` on macOS
`Ctrl+1` - `Ctrl+9` on Windows
`Alt+1` - `Alt+9` on Linux | Shortcut to open tab 1–9. | | `keymap.closeTab` | `Command+W` on macOS
`Ctrl+Shift+W` on Windows/Linux | Shortcut to close a tab. | @@ -458,6 +459,7 @@ Below is the list of the supported config properties. | `keymap.openProfiles` | `Command+I` on macOS
`Ctrl+Shift+I` on Windows/Linux | Shortcut to open the profile selector. | | `keymap.openSearchBar` | `Command+K` on macOS
`Ctrl+Shift+K` on Windows/Linux | Shortcut to open the search bar. | | `headless.skipConfirm` | false | Skips the confirmation prompt for Headless WebAuthn approval and instead prompts for WebAuthn immediately. | +| `ssh.noResume` | false | Disables SSH connection resumption. | { this.gracefullyKillTshdProcess(); }), - terminateWithTimeout(this.sharedProcess), + terminateWithTimeout(this.sharedProcess, 5_000, process => + // process.kill doesn't allow running a cleanup code in the child process + // on Windows + process.send(TERMINATE_MESSAGE) + ), this.agentRunner.killAll(), ]); } diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index 78e73e9fe1e90..be48373f056c2 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -276,3 +276,12 @@ export enum MainProcessIpc { export enum WindowsManagerIpc { SignalUserInterfaceReadiness = 'windows-manager-signal-user-interface-readiness', } + +/** + * A custom message to gracefully quit a process. + * It is sent to the child process with `process.send`. + * + * We need this because `process.kill('SIGTERM')` doesn't work on Windows, + * so we couldn't run any cleanup logic. + */ +export const TERMINATE_MESSAGE = 'TERMINATE_MESSAGE'; diff --git a/web/packages/teleterm/src/preload.ts b/web/packages/teleterm/src/preload.ts index c5b72720ca7a1..9a691d4ebf085 100644 --- a/web/packages/teleterm/src/preload.ts +++ b/web/packages/teleterm/src/preload.ts @@ -74,7 +74,14 @@ async function getElectronGlobals(): Promise { credentials.shared, runtimeSettings, { - noResume: mainProcessClient.configService.get('ssh.noResume').value, + ssh: { + noResume: mainProcessClient.configService.get('ssh.noResume').value, + }, + terminal: { + windowsBackend: mainProcessClient.configService.get( + 'terminal.windowsBackend' + ).value, + }, } ); const { diff --git a/web/packages/teleterm/src/services/config/appConfigSchema.ts b/web/packages/teleterm/src/services/config/appConfigSchema.ts index 91950eed522b1..bc8713886aa58 100644 --- a/web/packages/teleterm/src/services/config/appConfigSchema.ts +++ b/web/packages/teleterm/src/services/config/appConfigSchema.ts @@ -22,6 +22,9 @@ import { Platform } from 'teleterm/mainProcess/types'; import { createKeyboardShortcutSchema } from './keyboardShortcutSchema'; +// When adding a new config property, add it to the docs too +// (teleport-connect.mdx#configuration). + export type AppConfigSchema = ReturnType; export type AppConfig = z.infer; @@ -54,6 +57,12 @@ export const createAppConfigSchema = (platform: Platform) => { .max(256) .default(15) .describe('Font size for the terminal.'), + 'terminal.windowsBackend': z + .enum(['auto', 'winpty']) + .default('auto') + .describe( + '`auto` uses modern ConPTY system if available, which requires Windows 10 (19H1) or above. Set to `winpty` to use winpty even if ConPTY is available.' + ), 'usageReporting.enabled': z .boolean() .default(false) diff --git a/web/packages/teleterm/src/services/pty/fixtures/mocks.ts b/web/packages/teleterm/src/services/pty/fixtures/mocks.ts index c9e2c62e392c7..9cefda658c666 100644 --- a/web/packages/teleterm/src/services/pty/fixtures/mocks.ts +++ b/web/packages/teleterm/src/services/pty/fixtures/mocks.ts @@ -20,6 +20,7 @@ import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost'; import { PtyProcessCreationStatus, PtyServiceClient, + WindowsPty, } from 'teleterm/services/pty'; export class MockPtyProcess implements IPtyProcess { @@ -29,7 +30,7 @@ export class MockPtyProcess implements IPtyProcess { resize() {} - dispose() {} + async dispose() {} onData() { return () => {}; @@ -64,10 +65,12 @@ export class MockPtyServiceClient implements PtyServiceClient { createPtyProcess(): Promise<{ process: IPtyProcess; creationStatus: PtyProcessCreationStatus; + windowsPty: WindowsPty; }> { return Promise.resolve({ process: new MockPtyProcess(), creationStatus: PtyProcessCreationStatus.Ok, + windowsPty: undefined, }); } } diff --git a/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.test.ts b/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.test.ts index b37187ed4f5cd..4f8720d3d2482 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.test.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.test.ts @@ -47,7 +47,7 @@ describe('getPtyProcessOptions', () => { const { env } = getPtyProcessOptions( makeRuntimeSettings(), - { noResume: false }, + { ssh: { noResume: false }, windowsPty: { useConpty: true } }, cmd, processEnv ); @@ -76,7 +76,7 @@ describe('getPtyProcessOptions', () => { const { env } = getPtyProcessOptions( makeRuntimeSettings(), - { noResume: false }, + { ssh: { noResume: false }, windowsPty: { useConpty: true } }, cmd, processEnv ); @@ -103,7 +103,7 @@ describe('getPtyProcessOptions', () => { const { args } = getPtyProcessOptions( makeRuntimeSettings(), - { noResume: true }, + { ssh: { noResume: true }, windowsPty: { useConpty: true } }, cmd, processEnv ); diff --git a/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.ts b/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.ts index 4857d3a498694..e4b34c934b782 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/buildPtyOptions.ts @@ -25,8 +25,9 @@ import { assertUnreachable } from 'teleterm/ui/utils'; import { PtyCommand, PtyProcessCreationStatus, - SshOptions, TshKubeLoginCommand, + SshOptions, + WindowsPty, } from '../types'; import { @@ -34,9 +35,14 @@ import { ResolveShellEnvTimeoutError, } from './resolveShellEnv'; +type PtyOptions = { + ssh: SshOptions; + windowsPty: Pick; +}; + export async function buildPtyOptions( settings: RuntimeSettings, - sshOptions: SshOptions, + options: PtyOptions, cmd: PtyCommand ): Promise<{ processOptions: PtyProcessOptions; @@ -68,7 +74,7 @@ export async function buildPtyOptions( return { processOptions: getPtyProcessOptions( settings, - sshOptions, + options, cmd, combinedEnv ), @@ -79,10 +85,12 @@ export async function buildPtyOptions( export function getPtyProcessOptions( settings: RuntimeSettings, - sshOptions: SshOptions, + options: PtyOptions, cmd: PtyCommand, env: typeof process.env ): PtyProcessOptions { + const useConpty = options.windowsPty?.useConpty; + switch (cmd.kind) { case 'pty.shell': { // Teleport Connect bundles a tsh binary, but the user might have one already on their system. @@ -104,6 +112,7 @@ export function getPtyProcessOptions( cwd: cmd.cwd, env: { ...env, ...cmd.env }, initMessage: cmd.initMessage, + useConpty, }; } @@ -129,6 +138,7 @@ export function getPtyProcessOptions( path: settings.defaultShell, args: isWindows ? powershellCommandArgs : bashCommandArgs, env: { ...env, KUBECONFIG: getKubeConfigFilePath(cmd, settings) }, + useConpty, }; } @@ -140,7 +150,7 @@ export function getPtyProcessOptions( const args = [ `--proxy=${cmd.rootClusterId}`, 'ssh', - ...(sshOptions.noResume ? ['--no-resume'] : []), + ...(options.ssh.noResume ? ['--no-resume'] : []), '--forward-agent', loginHost, ]; @@ -149,6 +159,7 @@ export function getPtyProcessOptions( path: settings.tshd.binaryPath, args, env, + useConpty, }; } @@ -159,6 +170,7 @@ export function getPtyProcessOptions( path: cmd.path, args: cmd.args, env: { ...env, ...cmd.env }, + useConpty, }; } diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts index 74c9dfd90fa29..8990bfb0a1f64 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/ptyHostClient.ts @@ -41,6 +41,7 @@ export function createPtyHostClient( args: ptyOptions.args, path: ptyOptions.path, env: Struct.fromJson(ptyOptions.env), + useConpty: ptyOptions.useConpty, }); if (ptyOptions.cwd) { diff --git a/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts index 8b97c017f7a93..cec0ba3d246f3 100644 --- a/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts +++ b/web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts @@ -47,7 +47,7 @@ export function createPtyProcess( exchangeEventsStream.resize(columns, rows); }, - dispose(): void { + async dispose(): Promise { exchangeEventsStream.dispose(); }, diff --git a/web/packages/teleterm/src/services/pty/ptyHost/windowsPty.test.ts b/web/packages/teleterm/src/services/pty/ptyHost/windowsPty.test.ts new file mode 100644 index 0000000000000..28e89c7e26ccc --- /dev/null +++ b/web/packages/teleterm/src/services/pty/ptyHost/windowsPty.test.ts @@ -0,0 +1,61 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks'; + +import { getWindowsPty } from './windowsPty'; + +test.each([ + { + name: 'uses conpty on supported Windows version', + platform: 'win32' as const, + osVersion: '10.0.22621', + terminalOptions: { windowsBackend: 'auto' as const }, + expected: { useConpty: true, buildNumber: 22621 }, + }, + { + name: 'uses winpty on unsupported Windows version', + platform: 'win32' as const, + osVersion: '10.0.18308', + terminalOptions: { windowsBackend: 'auto' as const }, + expected: { useConpty: false, buildNumber: 18308 }, + }, + { + name: 'uses winpty when Windows version is supported, but conpty is disabled in options', + platform: 'win32' as const, + osVersion: '10.0.22621', + terminalOptions: { windowsBackend: 'winpty' as const }, + expected: { useConpty: false, buildNumber: 22621 }, + }, + { + name: 'undefined on non-Windows OS', + platform: 'darwin' as const, + osVersion: '23.5.0', + terminalOptions: { windowsBackend: 'auto' as const }, + expected: undefined, + }, +])('$name', ({ platform, osVersion, terminalOptions, expected }) => { + const pty = getWindowsPty( + makeRuntimeSettings({ + platform, + osVersion, + }), + terminalOptions + ); + expect(pty).toEqual(expected); +}); diff --git a/web/packages/teleterm/src/services/pty/ptyHost/windowsPty.ts b/web/packages/teleterm/src/services/pty/ptyHost/windowsPty.ts new file mode 100644 index 0000000000000..8bd69d0d9de32 --- /dev/null +++ b/web/packages/teleterm/src/services/pty/ptyHost/windowsPty.ts @@ -0,0 +1,49 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { RuntimeSettings } from 'teleterm/mainProcess/types'; + +import { TerminalOptions, WindowsPty } from '../types'; + +export const WIN_BUILD_STABLE_CONPTY = 18309; + +export function getWindowsPty( + runtimeSettings: RuntimeSettings, + terminalOptions: TerminalOptions +): WindowsPty { + if (runtimeSettings.platform !== 'win32') { + return undefined; + } + + const buildNumber = getWindowsBuildNumber(runtimeSettings.osVersion); + const useConpty = + terminalOptions.windowsBackend === 'auto' && + buildNumber >= WIN_BUILD_STABLE_CONPTY; + return { + useConpty, + buildNumber, + }; +} + +function getWindowsBuildNumber(osVersion: string): number { + const parsedOsVersion = /(\d+)\.(\d+)\.(\d+)/g.exec(osVersion); + if (parsedOsVersion?.length === 4) { + return parseInt(parsedOsVersion[3]); + } + return 0; +} diff --git a/web/packages/teleterm/src/services/pty/ptyService.ts b/web/packages/teleterm/src/services/pty/ptyService.ts index b13c11325179f..3b82021e8868d 100644 --- a/web/packages/teleterm/src/services/pty/ptyService.ts +++ b/web/packages/teleterm/src/services/pty/ptyService.ts @@ -23,21 +23,26 @@ import { RuntimeSettings } from 'teleterm/mainProcess/types'; import { buildPtyOptions } from './ptyHost/buildPtyOptions'; import { createPtyHostClient } from './ptyHost/ptyHostClient'; import { createPtyProcess } from './ptyHost/ptyProcess'; -import { PtyServiceClient, SshOptions } from './types'; +import { PtyServiceClient, PtyOptions } from './types'; +import { getWindowsPty } from './ptyHost/windowsPty'; export function createPtyService( address: string, credentials: ChannelCredentials, runtimeSettings: RuntimeSettings, - sshOptions: SshOptions + options: PtyOptions ): PtyServiceClient { const ptyHostClient = createPtyHostClient(address, credentials); return { createPtyProcess: async command => { + const windowsPty = getWindowsPty(runtimeSettings, options.terminal); const { processOptions, creationStatus } = await buildPtyOptions( runtimeSettings, - sshOptions, + { + ssh: options.ssh, + windowsPty, + }, command ); const ptyId = await ptyHostClient.createPtyProcess(processOptions); @@ -46,6 +51,7 @@ export function createPtyService( return { process: createPtyProcess(ptyHostClient, ptyId), creationStatus, + windowsPty, }; }, }; diff --git a/web/packages/teleterm/src/services/pty/types.ts b/web/packages/teleterm/src/services/pty/types.ts index 7119a89ea993c..aaac99a224ede 100644 --- a/web/packages/teleterm/src/services/pty/types.ts +++ b/web/packages/teleterm/src/services/pty/types.ts @@ -37,9 +37,21 @@ export type PtyServiceClient = { createPtyProcess: (cmd: PtyCommand) => Promise<{ process: IPtyProcess; creationStatus: PtyProcessCreationStatus; + windowsPty: WindowsPty; }>; }; +/** + * Pty information for Windows. + * undefined for non-Windows OS. + */ +export type WindowsPty = + | { + useConpty: boolean; + buildNumber: number; + } + | undefined; + export type ShellCommand = PtyCommandBase & { kind: 'pty.shell'; cwd?: string; @@ -107,3 +119,12 @@ export type SshOptions = { */ noResume: boolean; }; + +export type TerminalOptions = { + windowsBackend: 'auto' | 'winpty'; +}; + +export type PtyOptions = { + ssh: SshOptions; + terminal: TerminalOptions; +}; diff --git a/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto b/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto index f9baea4f2c9fc..98a0519c61a81 100644 --- a/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto +++ b/web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto @@ -41,6 +41,7 @@ message PtyCreate { reserved "init_command"; google.protobuf.Struct env = 7; string init_message = 8; + bool use_conpty = 9; } message PtyClientEvent { diff --git a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.ts b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.ts index cd6cd61badcea..35f043a2d4624 100644 --- a/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.ts +++ b/web/packages/teleterm/src/sharedProcess/api/protogen/ptyHostService_pb.ts @@ -69,6 +69,10 @@ export interface PtyCreate { * @generated from protobuf field: string init_message = 8; */ initMessage: string; + /** + * @generated from protobuf field: bool use_conpty = 9; + */ + useConpty: boolean; } /** * @generated from protobuf message PtyClientEvent @@ -266,7 +270,8 @@ class PtyCreate$Type extends MessageType { { no: 4, name: "args", kind: "scalar", repeat: 2 /*RepeatType.UNPACKED*/, T: 9 /*ScalarType.STRING*/ }, { no: 5, name: "cwd", kind: "scalar", T: 9 /*ScalarType.STRING*/ }, { no: 7, name: "env", kind: "message", T: () => Struct }, - { no: 8, name: "init_message", kind: "scalar", T: 9 /*ScalarType.STRING*/ } + { no: 8, name: "init_message", kind: "scalar", T: 9 /*ScalarType.STRING*/ }, + { no: 9, name: "use_conpty", kind: "scalar", T: 8 /*ScalarType.BOOL*/ } ]); } create(value?: PartialMessage): PtyCreate { @@ -275,6 +280,7 @@ class PtyCreate$Type extends MessageType { message.args = []; message.cwd = ""; message.initMessage = ""; + message.useConpty = false; if (value !== undefined) reflectionMergePartial(this, message, value); return message; @@ -299,6 +305,9 @@ class PtyCreate$Type extends MessageType { case /* string init_message */ 8: message.initMessage = reader.string(); break; + case /* bool use_conpty */ 9: + message.useConpty = reader.bool(); + break; default: let u = options.readUnknownField; if (u === "throw") @@ -326,6 +335,9 @@ class PtyCreate$Type extends MessageType { /* string init_message = 8; */ if (message.initMessage !== "") writer.tag(8, WireType.LengthDelimited).string(message.initMessage); + /* bool use_conpty = 9; */ + if (message.useConpty !== false) + writer.tag(9, WireType.Varint).bool(message.useConpty); let u = options.writeUnknownFields; if (u !== false) (u == true ? UnknownFieldHandler.onWrite : u)(this.typeName, message, writer); diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts index 0417408cc1ba7..a9aa281451131 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts @@ -131,16 +131,16 @@ export class PtyEventsStreamHandler { private handleStreamError(error: Error): void { this.logger.error(`stream has ended with error`, error); - this.cleanResources(); + void this.cleanResources(); } private handleStreamEnd(): void { this.logger.info(`stream has ended`); - this.cleanResources(); + void this.cleanResources(); } - private cleanResources(): void { - this.ptyProcess.dispose(); + private async cleanResources(): Promise { + await this.ptyProcess.dispose(); if (this.ptyId) { this.ptyProcesses.delete(this.ptyId); } diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyHostService.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyHostService.ts index 651a196e5ad6d..ffb1cb524321b 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyHostService.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyHostService.ts @@ -27,7 +27,9 @@ import { IPtyHost } from './../api/protogen/ptyHostService_pb.grpc-server'; import { PtyCwd, PtyId } from './../api/protogen/ptyHostService_pb'; import { PtyEventsStreamHandler } from './ptyEventsStreamHandler'; -export function createPtyHostService(): IPtyHost { +export function createPtyHostService(): IPtyHost & { + dispose(): Promise; +} { const logger = new Logger('PtyHostService'); const ptyProcesses = new Map(); @@ -43,6 +45,7 @@ export function createPtyHostService(): IPtyHost { ptyId, env: Struct.toJson(call.request.env!) as Record, initMessage: ptyOptions.initMessage, + useConpty: ptyOptions.useConpty, }); ptyProcesses.set(ptyId, ptyProcess); } catch (error) { @@ -73,5 +76,12 @@ export function createPtyHostService(): IPtyHost { }); }, exchangeEvents: stream => new PtyEventsStreamHandler(stream, ptyProcesses), + dispose: async () => { + await Promise.all( + Array.from(ptyProcesses.values()).map(ptyProcess => + ptyProcess.dispose() + ) + ); + }, }; } diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts index 18288c307d981..77d8a7dddad68 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts @@ -38,6 +38,7 @@ describe('PtyProcess', () => { args: [], env: { PATH: '/foo/bar' }, ptyId: '1234', + useConpty: true, }); const startErrorCb = jest.fn(); diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts index ededa80e5ffc4..f38e1503ffacf 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts @@ -24,6 +24,8 @@ import { EventEmitter } from 'node:events'; import * as nodePTY from 'node-pty'; import which from 'which'; +import { wait } from 'shared/utils/wait'; + import Logger from 'teleterm/logger'; import { PtyProcessOptions, IPtyProcess } from './types'; @@ -59,6 +61,12 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { * It emits TermEventEnum.StartError on error. start itself always returns a fulfilled promise. */ async start(cols: number, rows: number) { + if (process.platform === 'win32') { + this._logger.info( + this.options.useConpty ? 'ConPTY enabled' : 'ConPTY disabled' + ); + } + try { // which throws an error if the argument is not found in path. // TODO(ravicious): Remove the manual check for the existence of the executable after node-pty @@ -76,8 +84,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { // https://unix.stackexchange.com/questions/123858 cwd: this.options.cwd || getDefaultCwd(this.options.env), env: this.options.env, - // Turn off ConPTY due to an uncaught exception being thrown when a PTY is closed. - useConpty: false, + useConpty: this.options.useConpty, }); } catch (error) { this._logger.error(error); @@ -132,10 +139,38 @@ export class PtyProcess extends EventEmitter implements IPtyProcess { } } - dispose() { + async dispose() { + if (this._disposed) { + this._logger.info(`PTY process is not running. Nothing to kill`); + return; + } + const controller = new AbortController(); + const processExit = promisifyProcessExit(this._process); + this.removeAllListeners(); - this._process?.kill(); - this._disposed = true; + this._process.kill(); + + // Wait for the process to exit. + // It's needed for ssh sessions on Windows with ConPTY enabled. + // When we didn't wait, conhost.exe processes started by node-pty + // were left running after closing the app. + // Killing a process doesn't happen immediately, but instead appears to be + // queued, so we need to give it time to execute. + // + // Although this was added specifically for Windows, + // we run the same cleanup code for all platforms. + const hasExited = await Promise.race([ + processExit.then(() => controller.abort()).then(() => true), + // timeout for killing the shared process is 5 seconds + wait(4_000, controller.signal) + .catch(() => {}) // ignore abort errors + .then(() => false), + ]); + if (hasExited) { + this._disposed = true; + } else { + this._logger.error('Failed to dispose PTY process within the timeout'); + } } onData(cb: (data: string) => void) { @@ -254,3 +289,7 @@ function getDefaultCwd(env: Record): string { return userDir || process.cwd(); } + +function promisifyProcessExit(childProcess: nodePTY.IPty): Promise { + return new Promise(resolve => childProcess.onExit(() => resolve())); +} diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts index a43e82c033458..cd0f4b8bce4c6 100644 --- a/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts +++ b/web/packages/teleterm/src/sharedProcess/ptyHost/types.ts @@ -22,13 +22,15 @@ export type PtyProcessOptions = { args: string[]; cwd?: string; initMessage?: string; + /** Whether to use the ConPTY system on Windows. */ + useConpty: boolean; }; export type IPtyProcess = { start(cols: number, rows: number): void; write(data: string): void; resize(cols: number, rows: number): void; - dispose(): void; + dispose(): Promise; getCwd(): Promise; getPtyId(): string; // The listener removal functions are used only on the frontend app side from the renderer process. diff --git a/web/packages/teleterm/src/sharedProcess/sharedProcess.ts b/web/packages/teleterm/src/sharedProcess/sharedProcess.ts index b4f0f5c06c861..1b9be627d8f3e 100644 --- a/web/packages/teleterm/src/sharedProcess/sharedProcess.ts +++ b/web/packages/teleterm/src/sharedProcess/sharedProcess.ts @@ -28,7 +28,7 @@ import { readGrpcCert, shouldEncryptConnection, } from 'teleterm/services/grpcCredentials'; -import { RuntimeSettings } from 'teleterm/mainProcess/types'; +import { RuntimeSettings, TERMINATE_MESSAGE } from 'teleterm/mainProcess/types'; import Logger from 'teleterm/logger'; import { ptyHostDefinition } from 'teleterm/sharedProcess/api/protogen/ptyHostService_pb.grpc-server'; @@ -74,7 +74,8 @@ async function initializeServer( } const server = new Server(); - server.addService(ptyHostDefinition, createPtyHostService()); + const ptyHostService = createPtyHostService(); + server.addService(ptyHostDefinition, ptyHostService); // grpc-js requires us to pass localhost:port for TCP connections, const grpcServerAddress = address.replace('tcp://', ''); @@ -95,8 +96,13 @@ async function initializeServer( logger.error('Could not start shared server', e); } - process.once('exit', () => { - server.forceShutdown(); + process.on('message', async message => { + if (message === TERMINATE_MESSAGE) { + new Logger('Process').info('Received terminate message, exiting'); + server.forceShutdown(); + await ptyHostService.dispose(); + process.exit(0); + } }); } diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx index 440029df07578..15e53b53b36b6 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/DocumentTerminal.tsx @@ -132,6 +132,7 @@ export function DocumentTerminal(props: { unsanitizedFontFamily={unsanitizedTerminalFontFamily} fontSize={terminalFontSize} onEnterKey={attempt.data.refreshTitle} + windowsPty={attempt.data.windowsPty} /> )} diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx index b51605f63a810..fb6f2f4e7cc50 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/Terminal.tsx @@ -27,6 +27,7 @@ import { makeSuccessAttempt, } from 'shared/hooks/useAsync'; +import { WindowsPty } from 'teleterm/services/pty'; import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost'; import { DocumentTerminal } from 'teleterm/ui/services/workspacesService'; @@ -48,6 +49,7 @@ type TerminalProps = { unsanitizedFontFamily: string; fontSize: number; onEnterKey?(): void; + windowsPty: WindowsPty; }; export function Terminal(props: TerminalProps) { @@ -72,6 +74,7 @@ export function Terminal(props: TerminalProps) { el: refElement.current, fontSize: props.fontSize, theme: theme.colors.terminal, + windowsPty: props.windowsPty, }); // Start the PTY process. diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts index fdae12dcad53e..1af5663dda7f3 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts +++ b/web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts @@ -21,6 +21,7 @@ import { IDisposable, ITheme, Terminal } from 'xterm'; import { FitAddon } from 'xterm-addon-fit'; import { debounce } from 'shared/utils/highbar'; +import { WindowsPty } from 'teleterm/services/pty'; import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost'; import Logger from 'teleterm/logger'; @@ -30,6 +31,7 @@ type Options = { el: HTMLElement; fontSize: number; theme: ITheme; + windowsPty: WindowsPty; }; export default class TtyTerminal { @@ -68,6 +70,10 @@ export default class TtyTerminal { scrollback: 5000, minimumContrastRatio: 4.5, // minimum for WCAG AA compliance theme: this.options.theme, + windowsPty: this.options.windowsPty && { + backend: this.options.windowsPty.useConpty ? 'conpty' : 'winpty', + buildNumber: this.options.windowsPty.buildNumber, + }, windowOptions: { setWinSizeChars: true, }, diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx index 6aad2d0c4dceb..3280880988f4a 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx @@ -237,6 +237,7 @@ test('useDocumentTerminal shows a warning notification if the call to TerminalsS jest.spyOn(terminalsService, 'createPtyProcess').mockResolvedValue({ process: getPtyProcessMock(), creationStatus: PtyProcessCreationStatus.ResolveShellEnvTimeout, + windowsPty: undefined, }); jest.spyOn(notificationsService, 'notifyWarning'); @@ -574,6 +575,7 @@ const testSetup = ( return { process: getPtyProcessMock(), creationStatus: PtyProcessCreationStatus.Ok, + windowsPty: undefined, }; }); diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts index 029d7df11f38d..155cc3e36c200 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts +++ b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts @@ -29,7 +29,11 @@ import { import { IPtyProcess } from 'teleterm/sharedProcess/ptyHost'; import { useWorkspaceContext } from 'teleterm/ui/Documents'; import { routing } from 'teleterm/ui/uri'; -import { PtyCommand, PtyProcessCreationStatus } from 'teleterm/services/pty'; +import { + PtyCommand, + PtyProcessCreationStatus, + WindowsPty, +} from 'teleterm/services/pty'; import { AmbiguousHostnameError } from 'teleterm/ui/services/resources'; import { retryWithRelogin } from 'teleterm/ui/utils'; import Logger from 'teleterm/logger'; @@ -72,7 +76,7 @@ export function useDocumentTerminal(doc: types.DocumentTerminal) { return () => { if (attempt.status === 'success') { - attempt.data.ptyProcess.dispose(); + void attempt.data.ptyProcess.dispose(); } }; // This cannot be run only mount. If the user has initialized a new PTY process by clicking the @@ -230,7 +234,7 @@ async function setUpPtyProcess( getClusterName() ); - const ptyProcess = await createPtyProcess(ctx, cmd); + const { process: ptyProcess, windowsPty } = await createPtyProcess(ctx, cmd); if (doc.kind === 'doc.terminal_tsh_node') { ctx.usageService.captureProtocolUse({ @@ -317,14 +321,15 @@ async function setUpPtyProcess( ptyProcess, refreshTitle, openContextMenu, + windowsPty, }; } async function createPtyProcess( ctx: IAppContext, cmd: PtyCommand -): Promise { - const { process, creationStatus } = +): Promise<{ process: IPtyProcess; windowsPty: WindowsPty }> { + const { process, creationStatus, windowsPty } = await ctx.terminalsService.createPtyProcess(cmd); if (creationStatus === PtyProcessCreationStatus.ResolveShellEnvTimeout) { @@ -336,7 +341,7 @@ async function createPtyProcess( }); } - return process; + return { process, windowsPty }; } // TODO(ravicious): Instead of creating cmd within useDocumentTerminal, make useDocumentTerminal