Merge pull request #117292 from microsoft/tyriar/combined_id

Encode pty host id into terminal id

src/vs/platform/terminal/common/terminal.ts

@@ -96,7 +96,7 @@ export interface IPtyService {
 	): Promise<number>;
 	attachToProcess(id: number): Promise<void>;
 
-	start(id: number): Promise<ITerminalLaunchError | { persistentTerminalId: number; } | undefined>;
+	start(id: number): Promise<ITerminalLaunchError | undefined>;
 	shutdown(id: number, immediate: boolean): Promise<void>;
 	input(id: number, data: string): Promise<void>;
 	resize(id: number, cols: number, rows: number): Promise<void>;
@@ -243,6 +243,13 @@ export interface ITerminalLaunchError {
  * child_process.ChildProcess node.js interface.
  */
 export interface ITerminalChildProcess {
+	/**
+	 * A unique identifier for the terminal process. Note that the uniqueness only applies to a
+	 * given pty service connection, IDs will be duplicated for remote and local terminals for
+	 * example. The ID will be 0 if it does not support reconnection.
+	 */
+	id: number;
+
 	onProcessData: Event<IProcessDataEvent | string>;
 	onProcessExit: Event<number | undefined>;
 	onProcessReady: Event<{ pid: number, cwd: string }>;
@@ -256,7 +263,7 @@ export interface ITerminalChildProcess {
 	 * @returns undefined when the process was successfully started, otherwise an object containing
 	 * information on what went wrong.
 	 */
-	start(): Promise<ITerminalLaunchError | { persistentTerminalId: number } | undefined>;
+	start(): Promise<ITerminalLaunchError | undefined>;
 
 	/**
 	 * Shutdown the terminal process.

src/vs/platform/terminal/electron-browser/localPtyService.ts

@@ -15,9 +15,17 @@ import { LogLevelChannelClient } from 'vs/platform/log/common/logIpc';
 import { IGetTerminalLayoutInfoArgs, IPtyHostProcessReplayEvent, ISetTerminalLayoutInfoArgs } from 'vs/platform/terminal/common/terminalProcess';
 
 enum Constants {
-	MaxRestarts = 5
+	MaxRestarts = 5,
+	PtyHostIdMask = 0xFFFF0000,
+	RawTerminalIdMask = 0x0000FFFF,
+	PtyHostIdShift = 16
 }
 
+// Tracks the ID of the pty host, when a pty host gets restarted the new one is incremented in order
+// to differentiate the nth terminal from pty host #1 and #2. Since the pty host ID is encoded in
+// the regular ID, we can continue to share IPtyService's interface.
+let currentPtyHostId = 0;
+
 export class LocalPtyService extends Disposable implements IPtyService {
 	declare readonly _serviceBrand: undefined;
 
@@ -64,6 +72,7 @@ export class LocalPtyService extends Disposable implements IPtyService {
 	}
 
 	private _startPtyHost(): [Client, IPtyService] {
+		++currentPtyHostId;
 		const client = this._register(new Client(
 			FileAccess.asFileUri('bootstrap-fork', require).fsPath,
 			{
@@ -103,13 +112,13 @@ export class LocalPtyService extends Disposable implements IPtyService {
 
 		// Create proxy and forward events
 		const proxy = ProxyChannel.toService<IPtyService>(client.getChannel(TerminalIpcChannels.PtyHost));
-		this._register(proxy.onProcessData(e => this._onProcessData.fire(e)));
-		this._register(proxy.onProcessExit(e => this._onProcessExit.fire(e)));
-		this._register(proxy.onProcessReady(e => this._onProcessReady.fire(e)));
-		this._register(proxy.onProcessTitleChanged(e => this._onProcessTitleChanged.fire(e)));
-		this._register(proxy.onProcessOverrideDimensions(e => this._onProcessOverrideDimensions.fire(e)));
-		this._register(proxy.onProcessResolvedShellLaunchConfig(e => this._onProcessResolvedShellLaunchConfig.fire(e)));
-		this._register(proxy.onProcessReplay(e => this._onProcessReplay.fire(e)));
+		this._register(proxy.onProcessData(e => this._onProcessData.fire(this._convertEventToCombinedId(e))));
+		this._register(proxy.onProcessExit(e => this._onProcessExit.fire(this._convertEventToCombinedId(e))));
+		this._register(proxy.onProcessReady(e => this._onProcessReady.fire(this._convertEventToCombinedId(e))));
+		this._register(proxy.onProcessTitleChanged(e => this._onProcessTitleChanged.fire(this._convertEventToCombinedId(e))));
+		this._register(proxy.onProcessOverrideDimensions(e => this._onProcessOverrideDimensions.fire(this._convertEventToCombinedId(e))));
+		this._register(proxy.onProcessResolvedShellLaunchConfig(e => this._onProcessResolvedShellLaunchConfig.fire(this._convertEventToCombinedId(e))));
+		this._register(proxy.onProcessReplay(e => this._onProcessReplay.fire(this._convertEventToCombinedId(e))));
 
 		return [client, proxy];
 	}
@@ -123,42 +132,65 @@ export class LocalPtyService extends Disposable implements IPtyService {
 		const timeout = setTimeout(() => this._handleUnresponsiveCreateProcess(), HeartbeatConstants.CreateProcessTimeout);
 		const result = await this._proxy.createProcess(shellLaunchConfig, cwd, cols, rows, env, executableEnv, windowsEnableConpty, workspaceId, workspaceName);
 		clearTimeout(timeout);
-		return result;
+		console.log('new term combined id', this._getCombinedId(currentPtyHostId, result));
+		return this._getCombinedId(currentPtyHostId, result);
 	}
 
-	attachToProcess(id: number): Promise<void> {
-		return this._proxy.attachToProcess(id);
+	attachToProcess(combinedId: number): Promise<void> {
+		return this._proxy.attachToProcess(this._getTerminalIdOrThrow(combinedId));
 	}
 
-	start(id: number): Promise<ITerminalLaunchError | { persistentTerminalId: number; } | undefined> {
-		return this._proxy.start(id);
+	start(combinedId: number): Promise<ITerminalLaunchError | undefined> {
+		return this._proxy.start(this._getTerminalIdOrThrow(combinedId));
 	}
-	shutdown(id: number, immediate: boolean): Promise<void> {
-		return this._proxy.shutdown(id, immediate);
+	shutdown(combinedId: number, immediate: boolean): Promise<void> {
+		return this._proxy.shutdown(this._getTerminalIdOrThrow(combinedId), immediate);
 	}
-	input(id: number, data: string): Promise<void> {
-		return this._proxy.input(id, data);
+	input(combinedId: number, data: string): Promise<void> {
+		return this._proxy.input(this._getTerminalIdOrThrow(combinedId), data);
 	}
-	resize(id: number, cols: number, rows: number): Promise<void> {
-		return this._proxy.resize(id, cols, rows);
+	resize(combinedId: number, cols: number, rows: number): Promise<void> {
+		return this._proxy.resize(this._getTerminalIdOrThrow(combinedId), cols, rows);
 	}
-	acknowledgeDataEvent(id: number, charCount: number): Promise<void> {
-		return this._proxy.acknowledgeDataEvent(id, charCount);
+	acknowledgeDataEvent(combinedId: number, charCount: number): Promise<void> {
+		return this._proxy.acknowledgeDataEvent(this._getTerminalIdOrThrow(combinedId), charCount);
 	}
-	getInitialCwd(id: number): Promise<string> {
-		return this._proxy.getInitialCwd(id);
+	getInitialCwd(combinedId: number): Promise<string> {
+		return this._proxy.getInitialCwd(this._getTerminalIdOrThrow(combinedId));
 	}
-	getCwd(id: number): Promise<string> {
-		return this._proxy.getCwd(id);
+	getCwd(combinedId: number): Promise<string> {
+		return this._proxy.getCwd(this._getTerminalIdOrThrow(combinedId));
 	}
-	getLatency(id: number): Promise<number> {
-		return this._proxy.getLatency(id);
+	getLatency(combinedId: number): Promise<number> {
+		return this._proxy.getLatency(this._getTerminalIdOrThrow(combinedId));
 	}
 	setTerminalLayoutInfo(args: ISetTerminalLayoutInfoArgs): void {
+		for (const t of args.tabs) {
+			if (t.activePersistentTerminalId) {
+				t.activePersistentTerminalId = this._getRawTerminalId(t.activePersistentTerminalId);
+			}
+			for (const instance of t.terminals) {
+				instance.terminal = this._getRawTerminalId(instance.terminal);
+			}
+		}
 		return this._proxy.setTerminalLayoutInfo(args);
 	}
-	getTerminalLayoutInfo(args: IGetTerminalLayoutInfoArgs): Promise<ITerminalsLayoutInfo | undefined> {
-		return this._proxy.getTerminalLayoutInfo(args);
+	async getTerminalLayoutInfo(args: IGetTerminalLayoutInfoArgs): Promise<ITerminalsLayoutInfo | undefined> {
+		const result = await this._proxy.getTerminalLayoutInfo(args);
+		if (!result) {
+			return undefined;
+		}
+		for (const t of result.tabs) {
+			if (t.activePersistentTerminalId) {
+				t.activePersistentTerminalId = this._getCombinedId(currentPtyHostId, t.activePersistentTerminalId);
+			}
+			for (const instance of t.terminals) {
+				if (instance.terminal) {
+					instance.terminal.id = this._getCombinedId(currentPtyHostId, instance.terminal.id);
+				}
+			}
+		}
+		return result;
 	}
 
 	async restartPtyHost(): Promise<void> {
@@ -173,6 +205,34 @@ export class LocalPtyService extends Disposable implements IPtyService {
 		this._client.dispose();
 	}
 
+	private _getRawTerminalId(combinedId: number): number {
+		return combinedId & Constants.RawTerminalIdMask;
+	}
+
+	private _getPtyHostId(combinedId: number): number {
+		return (combinedId & Constants.PtyHostIdMask) >> Constants.PtyHostIdShift;
+	}
+
+	private _getCombinedId(ptyHostId: number, rawTerminalId: number): number {
+		return ((ptyHostId << Constants.PtyHostIdShift) | rawTerminalId) >>> 0/*force unsigned*/;
+	}
+
+	private _convertEventToCombinedId<T extends { id: number, event: any }>(e: T): T {
+		e.id = this._getCombinedId(currentPtyHostId, e.id);
+		return e;
+	}
+
+	/**
+	 * Verifies that the terminal's pty host ID is active and returns the raw terminal ID if so.
+	 */
+	private _getTerminalIdOrThrow(combinedId: number) {
+		if (currentPtyHostId !== this._getPtyHostId(combinedId)) {
+			console.log('ids', combinedId);
+			throw new Error(`Persistent terminal "${this._getRawTerminalId(combinedId)}": Pty host "${this._getPtyHostId(combinedId)}" is no longer active`);
+		}
+		return this._getRawTerminalId(combinedId);
+	}
+
 	private _handleHeartbeat() {
 		this._clearHeartbeatTimeouts();
 		this._heartbeatFirstTimeout = setTimeout(() => this._handleHeartbeatFirstTimeout(), HeartbeatConstants.BeatInterval * HeartbeatConstants.FirstWaitMultiplier);

src/vs/platform/terminal/node/ptyService.ts

@@ -13,7 +13,6 @@ import { TerminalProcess } from 'vs/platform/terminal/node/terminalProcess';
 import { ISetTerminalLayoutInfoArgs, ITerminalTabLayoutInfoDto, IPtyHostDescriptionDto, IGetTerminalLayoutInfoArgs, IPtyHostProcessReplayEvent } from 'vs/platform/terminal/common/terminalProcess';
 import { ILogService } from 'vs/platform/log/common/log';
 
-// TODO: On disconnect/restart, this will overwrite the older terminals
 let currentPtyId = 0;
 
 type WorkspaceId = string;
@@ -89,7 +88,7 @@ export class PtyService extends Disposable implements IPtyService {
 		this._logService.trace(`Persistent terminal "${id}": Attach`);
 	}
 
-	async start(id: number): Promise<ITerminalLaunchError | { persistentTerminalId: number; } | undefined> {
+	async start(id: number): Promise<ITerminalLaunchError | undefined> {
 		return this._throwIfNoPty(id).start();
 	}
 	async shutdown(id: number, immediate: boolean): Promise<void> {
@@ -257,10 +256,9 @@ export class PersistentTerminalProcess extends Disposable {
 		}));
 	}
 
-	async start(): Promise<ITerminalLaunchError | { persistentTerminalId: number } | undefined> {
-		let result;
+	async start(): Promise<ITerminalLaunchError | undefined> {
 		if (!this._isStarted) {
-			result = await this._terminalProcess.start();
+			const result = await this._terminalProcess.start();
 			if (result) {
 				// it's a terminal launch error
 				return result;
@@ -271,7 +269,7 @@ export class PersistentTerminalProcess extends Disposable {
 			this._onProcessTitleChanged.fire(this._terminalProcess.currentTitle);
 			this.triggerReplay();
 		}
-		return { persistentTerminalId: this._persistentTerminalId };
+		return undefined;
 	}
 	shutdown(immediate: boolean): void {
 		return this._terminalProcess.shutdown(immediate);

src/vs/platform/terminal/node/terminalProcess.ts

@@ -25,6 +25,8 @@ const WRITE_MAX_CHUNK_SIZE = 50;
 const WRITE_INTERVAL_MS = 5;
 
 export class TerminalProcess extends Disposable implements ITerminalChildProcess {
+	readonly id = 0;
+
 	private _exitCode: number | undefined;
 	private _exitMessage: string | undefined;
 	private _closeTimeout: any;

src/vs/workbench/api/common/extHostTerminalService.ts

@@ -184,6 +184,8 @@ export class ExtHostTerminal {
 }
 
 export class ExtHostPseudoterminal implements ITerminalChildProcess {
+	readonly id = 0;
+
 	private readonly _onProcessData = new Emitter<string>();
 	public readonly onProcessData: Event<string> = this._onProcessData.event;
 	private readonly _onProcessExit = new Emitter<number | undefined>();

src/vs/workbench/contrib/terminal/browser/remoteTerminalService.ts

@@ -107,7 +107,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP
 	private _inReplay = false;
 
 	constructor(
-		private readonly _terminalId: number,
+		readonly id: number,
 		private readonly _shellLaunchConfig: IShellLaunchConfig,
 		private readonly _activeWorkspaceRootUri: URI | undefined,
 		private readonly _cols: number,
@@ -130,7 +130,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP
 		}
 	}
 
-	public async start(): Promise<ITerminalLaunchError | { persistentTerminalId: number } | undefined> {
+	public async start(): Promise<ITerminalLaunchError | undefined> {
 		// Fetch the environment to check shell permissions
 		const env = await this._remoteAgentService.getEnvironment();
 		if (!env) {
@@ -149,7 +149,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP
 				env: this._shellLaunchConfig.env
 			};
 
-			this._logService.trace('Spawning remote agent process', { terminalId: this._terminalId, shellLaunchConfigDto });
+			this._logService.trace('Spawning remote agent process', { terminalId: this.id, shellLaunchConfigDto });
 
 			const result = await this._remoteTerminalChannel.createTerminalProcess(
 				shellLaunchConfigDto,
@@ -181,7 +181,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP
 		}
 
 		this._startBarrier.open();
-		return { persistentTerminalId: this._persistentTerminalId };
+		return undefined;
 	}
 
 	public shutdown(immediate: boolean): void {

src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts

@@ -15,6 +15,7 @@ import { IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensions, ITermin
 let hasReceivedResponseFromRemoteExtHost: boolean = false;
 
 export class TerminalProcessExtHostProxy extends Disposable implements ITerminalChildProcess, ITerminalProcessExtHostProxy {
+	readonly id = 0;
 
 	private readonly _onProcessData = this._register(new Emitter<string>());
 	public readonly onProcessData: Event<string> = this._onProcessData.event;

src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts

@@ -89,8 +89,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 	public get onEnvironmentVariableInfoChanged(): Event<IEnvironmentVariableInfo> { return this._onEnvironmentVariableInfoChange.event; }
 
 	public get environmentVariableInfo(): IEnvironmentVariableInfo | undefined { return this._environmentVariableInfo; }
-	private _persistentTerminalId: number | undefined;
-	public get persistentTerminalId(): number | undefined { return this._persistentTerminalId; }
+	public get persistentTerminalId(): number | undefined { return this._process?.id; }
 
 	public get hasWrittenData(): boolean {
 		return this._hasWrittenData;
@@ -236,9 +235,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 		}, LAUNCHING_DURATION);
 
 		const result = await this._process.start();
-		if (result && 'persistentTerminalId' in result) {
-			this._persistentTerminalId = result.persistentTerminalId;
-		} else if (result) {
+		if (result) {
 			// Error
 			return result;
 		}

src/vs/workbench/contrib/terminal/electron-sandbox/localPty.ts

@@ -32,18 +32,18 @@ export class LocalPty extends Disposable implements ITerminalChildProcess {
 	public readonly onProcessResolvedShellLaunchConfig = this._onProcessResolvedShellLaunchConfig.event;
 
 	constructor(
-		private readonly _localPtyId: number,
+		readonly id: number,
 		@ILocalPtyService private readonly _localPtyService: ILocalPtyService
 	) {
 		super();
-		this._localPtyService.onProcessData(e => e.id === this._localPtyId && this._onProcessData.fire(e.event));
-		this._localPtyService.onProcessExit(e => e.id === this._localPtyId && this._onProcessExit.fire(e.event));
-		this._localPtyService.onProcessReady(e => e.id === this._localPtyId && this._onProcessReady.fire(e.event));
-		this._localPtyService.onProcessTitleChanged(e => e.id === this._localPtyId && this._onProcessTitleChanged.fire(e.event));
-		this._localPtyService.onProcessOverrideDimensions(e => e.id === this._localPtyId && this._onProcessOverrideDimensions.fire(e.event));
-		this._localPtyService.onProcessResolvedShellLaunchConfig(e => e.id === this._localPtyId && this._onProcessResolvedShellLaunchConfig.fire(e.event));
+		this._localPtyService.onProcessData(e => e.id === this.id && this._onProcessData.fire(e.event));
+		this._localPtyService.onProcessExit(e => e.id === this.id && this._onProcessExit.fire(e.event));
+		this._localPtyService.onProcessReady(e => e.id === this.id && this._onProcessReady.fire(e.event));
+		this._localPtyService.onProcessTitleChanged(e => e.id === this.id && this._onProcessTitleChanged.fire(e.event));
+		this._localPtyService.onProcessOverrideDimensions(e => e.id === this.id && this._onProcessOverrideDimensions.fire(e.event));
+		this._localPtyService.onProcessResolvedShellLaunchConfig(e => e.id === this.id && this._onProcessResolvedShellLaunchConfig.fire(e.event));
 		this._localPtyService.onProcessReplay(e => {
-			if (e.id !== this._localPtyId) {
+			if (e.id !== this.id) {
 				return;
 			}
 			try {
@@ -73,38 +73,38 @@ export class LocalPty extends Disposable implements ITerminalChildProcess {
 		}
 	}
 
-	start(): Promise<ITerminalLaunchError | { persistentTerminalId: number; } | undefined> {
-		return this._localPtyService.start(this._localPtyId);
+	start(): Promise<ITerminalLaunchError | undefined> {
+		return this._localPtyService.start(this.id);
 	}
 	shutdown(immediate: boolean): void {
-		this._localPtyService.shutdown(this._localPtyId, immediate);
+		this._localPtyService.shutdown(this.id, immediate);
 	}
 	input(data: string): void {
 		if (this._inReplay) {
 			return;
 		}
-		this._localPtyService.input(this._localPtyId, data);
+		this._localPtyService.input(this.id, data);
 	}
 	resize(cols: number, rows: number): void {
 		if (this._inReplay) {
 			return;
 		}
-		this._localPtyService.resize(this._localPtyId, cols, rows);
+		this._localPtyService.resize(this.id, cols, rows);
 	}
 	getInitialCwd(): Promise<string> {
-		return this._localPtyService.getInitialCwd(this._localPtyId);
+		return this._localPtyService.getInitialCwd(this.id);
 	}
 	getCwd(): Promise<string> {
-		return this._localPtyService.getCwd(this._localPtyId);
+		return this._localPtyService.getCwd(this.id);
 	}
 	getLatency(): Promise<number> {
 		// TODO: The idea here was to add the result plus the time it took to get the latency
-		return this._localPtyService.getLatency(this._localPtyId);
+		return this._localPtyService.getLatency(this.id);
 	}
 	acknowledgeDataEvent(charCount: number): void {
 		if (this._inReplay) {
 			return;
 		}
-		this._localPtyService.acknowledgeDataEvent(this._localPtyId, charCount);
+		this._localPtyService.acknowledgeDataEvent(this.id, charCount);
 	}
 }