From 5ebb096db549b86442389d875ee5f6be2a6e7370 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 13 Apr 2021 15:34:59 -0500 Subject: [PATCH] Get terminals working - Instead of a single listener per terminal that handles all events VS Code now has a single listener per event that handles that event for all terminals. - Refactor Terminal to extend TerminalProcess to avoid duplicating methods. This required some modifications to TerminalProcess to access the pid and title and to set the ID. - Remove our async change to shutdown. This was necessary to avoid disposing too early but shutdown already calls dispose so it turns out we didn't need to call it ourselves. - Rename methods to match the command strings. - Fix getting system shell (uses process.env). - Use a single bufferer. Since it already supports buffering for multiple terminals there's no need to have one per terminal. - Remove replay/reconnect logic. It's broken and unused so there doesn't seem much point in trying to refactor it to fit the changes right now. While terminals work now there are still a lot of todos. --- .../platform/terminal/node/terminalProcess.ts | 10 +- lib/vscode/src/vs/server/node/channel.ts | 388 ++++++------------ 2 files changed, 134 insertions(+), 264 deletions(-) diff --git a/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts b/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts index c356a026..6c288ae4 100644 --- a/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts +++ b/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts @@ -45,14 +45,13 @@ const enum ShutdownConstants { } export class TerminalProcess extends Disposable implements ITerminalChildProcess { - readonly id = 0; readonly shouldPersist = false; private _exitCode: number | undefined; private _exitMessage: string | undefined; private _closeTimeout: any; - private _ptyProcess: pty.IPty | undefined; - private _currentTitle: string = ''; + protected _ptyProcess: pty.IPty | undefined; + protected _currentTitle: string = ''; private _processStartupComplete: Promise | undefined; private _isDisposed: boolean = false; private _windowsShellHelper: WindowsShellHelper | undefined; @@ -82,6 +81,7 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess public readonly onProcessShellTypeChanged = this._onProcessShellTypeChanged.event; constructor( + public readonly id: number = 0, private readonly _shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, @@ -293,9 +293,9 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess this._onProcessTitleChanged.fire(this._currentTitle); } - public async shutdown(immediate: boolean): Promise { + public shutdown(immediate: boolean): void { if (immediate) { - await this._kill(); + this._kill(); } else { if (!this._closeTimeout && !this._isDisposed) { this._queueProcessExit(); diff --git a/lib/vscode/src/vs/server/node/channel.ts b/lib/vscode/src/vs/server/node/channel.ts index 769ce639..5a7fa8e7 100644 --- a/lib/vscode/src/vs/server/node/channel.ts +++ b/lib/vscode/src/vs/server/node/channel.ts @@ -21,6 +21,10 @@ import { ILogService } from 'vs/platform/log/common/log'; import product from 'vs/platform/product/common/product'; import { IRemoteAgentEnvironment, RemoteAgentConnectionContext } from 'vs/platform/remote/common/remoteAgentEnvironment'; import { ITelemetryData, ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; +import { IProcessDataEvent, IShellLaunchConfig, ITerminalEnvironment, ITerminalLaunchError, ITerminalsLayoutInfo } from 'vs/platform/terminal/common/terminal'; +import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering'; +import { IGetTerminalLayoutInfoArgs, IProcessDetails, IPtyHostProcessReplayEvent, ISetTerminalLayoutInfoArgs } from 'vs/platform/terminal/common/terminalProcess'; +import { TerminalProcess } from 'vs/platform/terminal/node/terminalProcess'; import { getTranslations } from 'vs/server/node/nls'; import { getUriTransformer } from 'vs/server/node/util'; import { IFileChangeDto } from 'vs/workbench/api/common/extHost.protocol'; @@ -28,12 +32,8 @@ import { IEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/co import { MergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableCollection'; import { deserializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared'; import * as terminal from 'vs/workbench/contrib/terminal/common/remoteTerminalChannel'; -import { IShellLaunchConfig, ITerminalEnvironment, ITerminalLaunchError, ITerminalsLayoutInfo } from 'vs/platform/terminal/common/terminal'; -import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering'; import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; import { getMainProcessParentEnv } from 'vs/workbench/contrib/terminal/node/terminalEnvironment'; -import { TerminalProcess } from 'vs/platform/terminal/node/terminalProcess'; -import { ISetTerminalLayoutInfoArgs, IGetTerminalLayoutInfoArgs, IProcessDetails } from 'vs/platform/terminal/common/terminalProcess'; import { AbstractVariableResolverService } from 'vs/workbench/services/configurationResolver/common/variableResolver'; import { ExtensionScanner, ExtensionScannerInput } from 'vs/workbench/services/extensions/node/extensionPoints'; @@ -386,239 +386,47 @@ class VariableResolverService extends AbstractVariableResolverService { } } -class Terminal { - private readonly process: TerminalProcess; - private _pid: number = -1; - private _title: string = ''; - public readonly workspaceId: string; - public readonly workspaceName: string; - private readonly persist: boolean; +class Terminal extends TerminalProcess { + private readonly workspaceId: string; + private readonly workspaceName: string; - private readonly _onDispose = new Emitter(); - public get onDispose(): Event { return this._onDispose.event; } + // TODO: Implement once we have persistent terminals. + public isOrphan: boolean = false; - private _isOrphan = true; - public get isOrphan(): boolean { return this._isOrphan; } - - // These are replayed when a client reconnects. - private cols: number; - private rows: number; - private replayData: string[] = []; - // This is based on string length and is pretty arbitrary. - private readonly maxReplayData = 10000; - private totalReplayData = 0; - - // According to the release notes the terminals are supposed to dispose after - // a short timeout; in our case we'll use 48 hours so you can get them back - // the next day or over the weekend. - private disposeTimeout: NodeJS.Timeout | undefined; - private disposeDelay = 48 * 60 * 60 * 1000; - - private buffering = false; - private readonly _onEvent = new Emitter({ - // Don't bind to data until something is listening. - onFirstListenerAdd: () => { - logger.debug('Terminal bound', field('id', this.id)); - this._isOrphan = false; - if (!this.buffering) { - this.buffering = true; - this.bufferer.startBuffering(this.id, this.process.onProcessData); - } - }, - - // Replay stored events. - onFirstListenerDidAdd: () => { - // We only need to replay if the terminal is being reconnected which is - // true if there is a dispose timeout. - if (typeof this.disposeTimeout !== 'undefined') { - return; - } - - clearTimeout(this.disposeTimeout); - this.disposeTimeout = undefined; - - logger.debug('Terminal replaying', field('id', this.id)); - this._onEvent.fire({ - type: 'replay', - events: [{ - cols: this.cols, - rows: this.rows, - data: this.replayData.join(''), - }] - }); - }, - - onLastListenerRemove: () => { - logger.debug('Terminal unbound', field('id', this.id)); - this._isOrphan = true; - if (!this.persist) { // Used by debug consoles. - this.dispose(); - } else { - this.disposeTimeout = setTimeout(() => { - this.dispose(); - }, this.disposeDelay); - } - } - }); - - public get onEvent(): Event { return this._onEvent.event; } - - // Buffer to reduce the number of messages going to the renderer. - private readonly bufferer = new TerminalDataBufferer((_, data) => { - this._onEvent.fire({ - type: 'data', - data, - }); - - // No need to store data if we aren't persisting. - if (!this.persist) { - return; - } - - this.replayData.push(data); - this.totalReplayData += data.length; - - let overflow = this.totalReplayData - this.maxReplayData; - if (overflow <= 0) { - return; - } - - // Drop events until doing so would put us under budget. - let deleteCount = 0; - for (; deleteCount < this.replayData.length - && this.replayData[deleteCount].length <= overflow; ++deleteCount) { - overflow -= this.replayData[deleteCount].length; - } - - if (deleteCount > 0) { - this.replayData.splice(0, deleteCount); - } - - // Dropping any more events would put us under budget; trim the first event - // instead if still over budget. - if (overflow > 0 && this.replayData.length > 0) { - this.replayData[0] = this.replayData[0].substring(overflow); - } - - this.totalReplayData = this.replayData.reduce((p, c) => p + c.length, 0); - }); - - public get pid(): number { - return this._pid; - } - - public get title(): string { - return this._title; - } + public get title(): string { return this._currentTitle; } + public get pid(): number { return this._ptyProcess?.pid ?? -1; } public constructor( - public readonly id: number, + id: number, config: IShellLaunchConfig & { cwd: string }, args: terminal.ICreateTerminalProcessArguments, env: platform.IProcessEnvironment, logService: ILogService, ) { - this.workspaceId = args.workspaceId; - this.workspaceName = args.workspaceName; - - this.cols = args.cols; - this.rows = args.rows; - - // TODO: Don't persist terminals until we make it work with things like - // htop, vim, etc. - // this.persist = args.shouldPersistTerminal; - this.persist = false; - - this.process = new TerminalProcess( + super( + id, config, config.cwd, - this.cols, - this.rows, + args.cols, + args.rows, env, process.env as platform.IProcessEnvironment, // Environment used for `findExecutable`. false, // windowsEnableConpty: boolean, logService, ); - // The current pid and title aren't exposed so they have to be tracked. - this.process.onProcessReady((event) => { - this._pid = event.pid; - this._onEvent.fire({ - type: 'ready', - pid: event.pid, - cwd: event.cwd, - }); - }); + this.workspaceId = args.workspaceId; + this.workspaceName = args.workspaceName; - this.process.onProcessTitleChanged((title) => { - this._title = title; - this._onEvent.fire({ - type: 'titleChanged', - title, - }); - }); - - this.process.onProcessExit((exitCode) => { - logger.debug('Terminal exited', field('id', this.id), field('code', exitCode)); - this._onEvent.fire({ - type: 'exit', - exitCode, - }); - this.dispose(); - }); - - // TODO: I think `execCommand` must have something to do with running - // commands on the terminal that will do things in VS Code but we already - // have that functionality via a socket so I'm not sure what this is for. - // type: 'execCommand'; - // reqId: number; - // commandId: string; - // commandArgs: any[]; - - // TODO: Maybe this is to ask if the terminal is currently attached to - // anything? But we already know that on account of whether anything is - // listening to our event emitter. - // type: 'orphan?'; + // Ensure other listeners run before disposing the emitters. + this.onProcessExit(() => setTimeout(() => this.shutdown(true), 1)); } - public async dispose() { - logger.debug('Terminal disposing', field('id', this.id)); - this._onEvent.dispose(); - this.bufferer.dispose(); - await this.process.shutdown(true); - this.process.dispose(); - this._onDispose.fire(); - this._onDispose.dispose(); - } - - public shutdown(immediate: boolean): Promise { - return this.process.shutdown(immediate); - } - - public getCwd(): Promise { - return this.process.getCwd(); - } - - public getInitialCwd(): Promise { - return this.process.getInitialCwd(); - } - - public start(): Promise { - return this.process.start(); - } - - public input(data: string): void { - return this.process.input(data); - } - - public acknowledgeDataEvent(charCount: number): void { - return this.process.acknowledgeDataEvent(charCount); - } - - public resize(cols: number, rows: number): void { - this.cols = cols; - this.rows = rows; - return this.process.resize(cols, rows); + /** + * Run `fn` when the terminal disposes. + */ + public onDispose(dispose: () => void) { + this._register({ dispose }); } /** @@ -646,35 +454,74 @@ export class TerminalProviderChannel implements IServerChannel(); - public constructor (private readonly logService: ILogService) { + // These re-emit events from terminals with their IDs attached. + private readonly _onProcessData = new Emitter<{ id: number, event: IProcessDataEvent | string }>({ + // Shut down all terminals when all clients disconnect. Unfortunately this + // means that if you open multiple tabs and close one the terminals spawned + // by that tab won't shut down and they can't be reconnected either since we + // don't have persistence yet. + // TODO: Implement persistence. + onLastListenerRemove: () => { + this.terminals.forEach((t) => t.shutdown(true)); + } + }); + private readonly _onProcessExit = new Emitter<{ id: number, event: number | undefined }>(); + private readonly _onProcessReady = new Emitter<{ id: number, event: { pid: number, cwd: string } }>(); + private readonly _onProcessReplay = new Emitter<{ id: number, event: IPtyHostProcessReplayEvent }>(); + private readonly _onProcessTitleChanged = new Emitter<{ id: number, event: string }>(); - } + // Buffer to reduce the number of messages going to the renderer. + private readonly bufferer = new TerminalDataBufferer((id, data) => { + // TODO: Not sure what sync means. + this._onProcessData.fire({ id, event: { data, sync: true }}); + }); + + public constructor (private readonly logService: ILogService) {} + + public listen(_: RemoteAgentConnectionContext, event: string, args: any): Event { + logger.trace('TerminalProviderChannel:listen', field("event", event), field("args", args)); - public listen(_: RemoteAgentConnectionContext, event: string, args?: any): Event { switch (event) { - case '$onTerminalProcessEvent': return this.onTerminalProcessEvent(args); + case '$onPtyHostExitEvent': return Event.None; // TODO + case '$onPtyHostStartEvent': return Event.None; // TODO + case '$onPtyHostUnresponsiveEvent': return Event.None; // TODO + case '$onPtyHostResponsiveEvent': return Event.None; // TODO + + case '$onProcessDataEvent': return this._onProcessData.event; + case '$onProcessExitEvent': return this._onProcessExit.event; + case '$onProcessReadyEvent': return this._onProcessReady.event; + case '$onProcessReplayEvent': return this._onProcessReplay.event; + case '$onProcessTitleChangedEvent': return this._onProcessTitleChanged.event; + case '$onProcessShellTypeChangedEvent': return Event.None; // TODO; + case '$onProcessOverrideDimensionsEvent': return Event.None; // TODO; + case '$onProcessResolvedShellLaunchConfigEvent': return Event.None; // TODO; + case '$onProcessOrphanQuestion': return Event.None; // TODO + // TODO: I think this must have something to do with running commands on + // the terminal that will do things in VS Code but we already have that + // functionality via a socket so I'm not sure what this is for. + case '$onExecuteCommand': return Event.None; } throw new Error(`Invalid listen '${event}'`); } - private onTerminalProcessEvent(id: number): Event { - return this.getTerminal(id).onEvent; - } + public call(context: RemoteAgentConnectionContext, command: string, args: any): Promise { + logger.trace('TerminalProviderChannel:call', field("command", command), field("args", args)); - public call(context: RemoteAgentConnectionContext, command: string, args?: any): Promise { switch (command) { - case '$createProcess': return this.createTerminalProcess(context.remoteAuthority, args); - case '$start': return this.startTerminalProcess(...args as [number]); - case '$input': return this.sendInputToTerminalProcess(...args as [number, string]); - case '$acknowledgeDataEvent': return this.sendCharCountToTerminalProcess(...args as [number, number]); - case '$shutdown': return this.shutdownTerminalProcess(...args as [number, boolean]); - case '$resize': return this.resizeTerminalProcess(...args as [number, number, number]); - case '$getInitialCwd': return this.getTerminalInitialCwd(...args as [number]); - case '$getCwd': return this.getTerminalCwd(...args as [number]); - case '$sendCommandResult': return this.sendCommandResultToTerminalProcess(...args as [number, number, boolean, any]); - case '$orphanQuestionReply': return this.orphanQuestionReply(...args as [number]); - case '$listProcesses': return this.listProcesses(); + case '$restartPtyHost': return this.restartPtyHost(); + case '$createProcess': return this.createProcess(context.remoteAuthority, args); + case '$attachProcess': return this.attachProcess(args[0]); + case '$start': return this.start(args[0]); + case '$input': return this.input(args[0], args[1]); + case '$acknowledgeDataEvent': return this.acknowledgeDataEvent(args[0], args[1]); + case '$shutdown': return this.shutdown(args[0], args[1]); + case '$resize': return this.resize(args[0], args[1], args[2]); + case '$getInitialCwd': return this.getInitialCwd(args[0]); + case '$getCwd': return this.getCwd(args[0]); + case '$sendCommandResult': return this.sendCommandResult(args[0], args[1], args[2], args[3]); + case '$orphanQuestionReply': return this.orphanQuestionReply(args[0]); + case '$listProcesses': return this.listProcesses(args[0]); case '$setTerminalLayoutInfo': return this.setTerminalLayoutInfo(args); case '$getTerminalLayoutInfo': return this.getTerminalLayoutInfo(args); } @@ -683,10 +530,15 @@ export class TerminalProviderChannel implements IServerChannel t.dispose()); + this.bufferer.dispose(); + this.terminals.forEach((t) => t.shutdown(false)); } - private async createTerminalProcess(remoteAuthority: string, args: terminal.ICreateTerminalProcessArguments): Promise { + private async restartPtyHost(): Promise { + throw new Error('TODO: restartPtyHost'); + } + + private async createProcess(remoteAuthority: string, args: terminal.ICreateTerminalProcessArguments): Promise { const terminalId = this.id++; logger.debug('Creating terminal', field('id', terminalId), field('terminals', this.terminals.size)); @@ -729,7 +581,7 @@ export class TerminalProviderChannel implements IServerChannel args.configuration[key], args.isWorkspaceShellAllowed, - await getSystemShell(platform.platform, env), + await getSystemShell(platform.platform, process.env as platform.IProcessEnvironment), process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), process.env.windir, resolver, @@ -811,12 +663,28 @@ export class TerminalProviderChannel implements IServerChannel this.terminals.delete(terminalId)); + this.terminals.set(terminal.id, terminal); + logger.debug('Created terminal', field('id', terminal.id)); + terminal.onDispose(() => { + this.terminals.delete(terminal.id); + this.bufferer.stopBuffering(terminal.id); + }); + + // Hook up terminal events to the global event emitter. + this.bufferer.startBuffering(terminal.id, terminal.onProcessData); + terminal.onProcessExit((exitCode) => { + logger.debug('Terminal exited', field('id', terminal.id), field('code', exitCode)); + this._onProcessExit.fire({ id: terminal.id, event: exitCode }); + }); + terminal.onProcessReady((event) => { + this._onProcessReady.fire({ id: terminal.id, event }); + }); + terminal.onProcessTitleChanged((title) => { + this._onProcessTitleChanged.fire({ id: terminal.id, event: title }); + }); return { - persistentTerminalId: terminalId, + persistentTerminalId: terminal.id, resolvedShellLaunchConfig, }; } @@ -829,49 +697,51 @@ export class TerminalProviderChannel implements IServerChannel { + private async attachProcess(_id: number): Promise { + // TODO: Won't be necessary until we have persistent terminals. + throw new Error('not implemented'); + } + + private async start(id: number): Promise { return this.getTerminal(id).start(); } - private async sendInputToTerminalProcess(id: number, data: string): Promise { + private async input(id: number, data: string): Promise { return this.getTerminal(id).input(data); } - private async sendCharCountToTerminalProcess(id: number, charCount: number): Promise { + private async acknowledgeDataEvent(id: number, charCount: number): Promise { return this.getTerminal(id).acknowledgeDataEvent(charCount); } - private async shutdownTerminalProcess(id: number, immediate: boolean): Promise { + private async shutdown(id: number, immediate: boolean): Promise { return this.getTerminal(id).shutdown(immediate); } - private async resizeTerminalProcess(id: number, cols: number, rows: number): Promise { + private async resize(id: number, cols: number, rows: number): Promise { return this.getTerminal(id).resize(cols, rows); } - private async getTerminalInitialCwd(id: number): Promise { + private async getInitialCwd(id: number): Promise { return this.getTerminal(id).getInitialCwd(); } - private async getTerminalCwd(id: number): Promise { + private async getCwd(id: number): Promise { return this.getTerminal(id).getCwd(); } - private async sendCommandResultToTerminalProcess(id: number, reqId: number, isError: boolean, payload: any): Promise { - // NOTE: Not required unless we implement the `execCommand` event, see above. + private async sendCommandResult(_id: number, _reqId: number, _isError: boolean, _payload: any): Promise { + // NOTE: Not required unless we implement the matching event, see above. throw new Error('not implemented'); } - private async orphanQuestionReply(id: number): Promise { - // NOTE: Not required unless we implement the `orphan?` event, see above. + private async orphanQuestionReply(_id: number): Promise { + // NOTE: Not required unless we implement the matching event, see above. throw new Error('not implemented'); } - private async listProcesses(): Promise { - // TODO: args.isInitialization. Maybe this is to have slightly different - // behavior when first listing terminals but I don't know what you'd want to - // do differently. Maybe it's to reset the terminal dispose timeouts or - // something like that, but why not do it each time you list? + private async listProcesses(_reduceGraceTime: boolean): Promise { + // TODO: reduceGraceTime. const terminals = await Promise.all(Array.from(this.terminals).map(async ([id, terminal]) => { return terminal.description(id); }));