Call ctor with port so proxy is non-undefined

Author: Tyriar

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

@@ -273,7 +273,7 @@ export interface IPtyHostController {
 	readonly onPtyHostResponsive?: Event<void>;
 	readonly onPtyHostRequestResolveVariables?: Event<IRequestResolveVariablesEvent>;
 
-	restartPtyHost?(): Promise<void>;
+	restartPtyHost?(): void;
 	acceptPtyHostResolvedVariables?(requestId: number, resolved: string[]): Promise<void>;
 }
 

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

@@ -312,9 +312,9 @@ export class PtyHostService extends Disposable implements IPtyService {
 	}
 
 	async restartPtyHost(): Promise<void> {
-		this._isResponsive = true;
 		this._disposePtyHost();
-		[this._connection, this._proxy] = await this._startPtyHost();
+		this._isResponsive = true;
+		[this._connection, this._proxy] = this._startPtyHost();
 	}
 
 	private _disposePtyHost(): void {

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

@@ -39,24 +39,34 @@ import { mark } from 'vs/base/common/performance';
 export class LocalTerminalBackendContribution implements IWorkbenchContribution {
 	constructor(
 		@IInstantiationService instantiationService: IInstantiationService,
+		@ILogService logService: ILogService,
 		@ITerminalInstanceService terminalInstanceService: ITerminalInstanceService
 	) {
-		const backend = instantiationService.createInstance(LocalTerminalBackend, undefined);
-		Registry.as<ITerminalBackendRegistry>(TerminalExtensions.Backend).registerTerminalBackend(backend);
-		terminalInstanceService.didRegisterBackend(backend.remoteAuthority);
+		mark('code/willConnectPtyHost');
+		logService.trace('Renderer->PtyHost#connect: before acquirePort');
+		acquirePort('vscode:createPtyHostMessageChannel', 'vscode:createPtyHostMessageChannelResult').then(port => {
+			mark('code/didConnectPtyHost');
+			logService.trace('Renderer->PtyHost#connect: connection established');
+
+			const backend = instantiationService.createInstance(LocalTerminalBackend, port);
+			Registry.as<ITerminalBackendRegistry>(TerminalExtensions.Backend).registerTerminalBackend(backend);
+			terminalInstanceService.didRegisterBackend(backend.remoteAuthority);
+		});
 	}
 }
 
 class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBackend {
+	readonly remoteAuthority = undefined;
+
 	private readonly _ptys: Map<number, LocalPty> = new Map();
 
-	private _ptyHostDirectProxy: IPtyService | undefined;
+	private _ptyHostDirectProxy: IPtyService;
 
 	private readonly _onDidRequestDetach = this._register(new Emitter<{ requestId: number; workspaceId: string; instanceId: number }>());
 	readonly onDidRequestDetach = this._onDidRequestDetach.event;
 
 	constructor(
-		readonly remoteAuthority: string | undefined,
+		port: MessagePort,
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@IWorkspaceContextService workspaceContextService: IWorkspaceContextService,
 		@ILogService logService: ILogService,
@@ -76,90 +86,81 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
 	) {
 		super(_localPtyService, logService, notificationService, historyService, _configurationResolverService, workspaceContextService);
 
-		// TODO: If we connect async like this, there may be a race condition where messages go missing before the message port is established
-		this._start();
-	}
-
-	private async _start() {
-
-		// TODO: Use the direct connection
-		mark('code/willConnectPtyHost');
-		this._logService.trace('Renderer->PtyHost#connect: before acquirePort');
-		const port = await acquirePort('vscode:createPtyHostMessageChannel', 'vscode:createPtyHostMessageChannelResult');
-		mark('code/didConnectPtyHost');
-		this._logService.trace('Renderer->PtyHost#connect: connection established');
-
+		// There are two connections to the pty host; one to the regular shared process
+		// _localPtyService, and one directly via message port _ptyHostDirectProxy. The former is
+		// used for pty host management messages, it would make sense in the future to use a
+		// separate interface/service for this one.
 		const client = new MessagePortClient(port, `window:${this._environmentService.window.id}`);
 		this._ptyHostDirectProxy = ProxyChannel.toService<IPtyService>(client.getChannel(TerminalIpcChannels.PtyHostWindow));
 
 		// Attach process listeners
-		this._ptyHostDirectProxy!.onProcessData(e => this._ptys.get(e.id)?.handleData(e.event));
-		this._ptyHostDirectProxy!.onDidChangeProperty(e => this._ptys.get(e.id)?.handleDidChangeProperty(e.property));
-		this._ptyHostDirectProxy!.onProcessExit(e => {
+		this._ptyHostDirectProxy.onProcessData(e => this._ptys.get(e.id)?.handleData(e.event));
+		this._ptyHostDirectProxy.onDidChangeProperty(e => this._ptys.get(e.id)?.handleDidChangeProperty(e.property));
+		this._ptyHostDirectProxy.onProcessExit(e => {
 			const pty = this._ptys.get(e.id);
 			if (pty) {
 				pty.handleExit(e.event);
 				this._ptys.delete(e.id);
 			}
 		});
-		this._ptyHostDirectProxy!.onProcessReady(e => this._ptys.get(e.id)?.handleReady(e.event));
-		this._ptyHostDirectProxy!.onProcessReplay(e => this._ptys.get(e.id)?.handleReplay(e.event));
-		this._ptyHostDirectProxy!.onProcessOrphanQuestion(e => this._ptys.get(e.id)?.handleOrphanQuestion());
-		this._ptyHostDirectProxy!.onDidRequestDetach(e => this._onDidRequestDetach.fire(e));
+		this._ptyHostDirectProxy.onProcessReady(e => this._ptys.get(e.id)?.handleReady(e.event));
+		this._ptyHostDirectProxy.onProcessReplay(e => this._ptys.get(e.id)?.handleReplay(e.event));
+		this._ptyHostDirectProxy.onProcessOrphanQuestion(e => this._ptys.get(e.id)?.handleOrphanQuestion());
+		this._ptyHostDirectProxy.onDidRequestDetach(e => this._onDidRequestDetach.fire(e));
 
 		// Listen for config changes
 		const initialConfig = this._configurationService.getValue<ITerminalConfiguration>(TERMINAL_CONFIG_SECTION);
 		for (const match of Object.keys(initialConfig.autoReplies)) {
 			// Ensure the reply is value
 			const reply = initialConfig.autoReplies[match] as string | null;
 			if (reply) {
-				this._ptyHostDirectProxy!.installAutoReply(match, reply);
+				this._ptyHostDirectProxy.installAutoReply(match, reply);
 			}
 		}
 		// TODO: Could simplify update to a single call
 		this._register(this._configurationService.onDidChangeConfiguration(async e => {
 			if (e.affectsConfiguration(TerminalSettingId.AutoReplies)) {
-				this._ptyHostDirectProxy!.uninstallAllAutoReplies();
+				this._ptyHostDirectProxy.uninstallAllAutoReplies();
 				const config = this._configurationService.getValue<ITerminalConfiguration>(TERMINAL_CONFIG_SECTION);
 				for (const match of Object.keys(config.autoReplies)) {
 					// Ensure the reply is value
 					const reply = config.autoReplies[match] as string | null;
 					if (reply) {
-						await this._ptyHostDirectProxy!.installAutoReply(match, reply);
+						await this._ptyHostDirectProxy.installAutoReply(match, reply);
 					}
 				}
 			}
 		}));
 	}
 
 	async requestDetachInstance(workspaceId: string, instanceId: number): Promise<IProcessDetails | undefined> {
-		return this._ptyHostDirectProxy!.requestDetachInstance(workspaceId, instanceId);
+		return this._ptyHostDirectProxy.requestDetachInstance(workspaceId, instanceId);
 	}
 
 	async acceptDetachInstanceReply(requestId: number, persistentProcessId?: number): Promise<void> {
 		if (!persistentProcessId) {
 			this._logService.warn('Cannot attach to feature terminals, custom pty terminals, or those without a persistentProcessId');
 			return;
 		}
-		return this._ptyHostDirectProxy!.acceptDetachInstanceReply(requestId, persistentProcessId);
+		return this._ptyHostDirectProxy.acceptDetachInstanceReply(requestId, persistentProcessId);
 	}
 
 	async persistTerminalState(): Promise<void> {
 		const ids = Array.from(this._ptys.keys());
-		const serialized = await this._ptyHostDirectProxy!.serializeTerminalState(ids);
+		const serialized = await this._ptyHostDirectProxy.serializeTerminalState(ids);
 		this._storageService.store(TerminalStorageKeys.TerminalBufferState, serialized, StorageScope.WORKSPACE, StorageTarget.MACHINE);
 	}
 
 	async updateTitle(id: number, title: string, titleSource: TitleEventSource): Promise<void> {
-		await this._ptyHostDirectProxy!.updateTitle(id, title, titleSource);
+		await this._ptyHostDirectProxy.updateTitle(id, title, titleSource);
 	}
 
 	async updateIcon(id: number, userInitiated: boolean, icon: URI | { light: URI; dark: URI } | { id: string; color?: { id: string } }, color?: string): Promise<void> {
-		await this._ptyHostDirectProxy!.updateIcon(id, userInitiated, icon, color);
+		await this._ptyHostDirectProxy.updateIcon(id, userInitiated, icon, color);
 	}
 
 	updateProperty<T extends ProcessPropertyType>(id: number, property: ProcessPropertyType, value: IProcessPropertyMap[T]): Promise<void> {
-		return this._ptyHostDirectProxy!.updateProperty(id, property, value);
+		return this._ptyHostDirectProxy.updateProperty(id, property, value);
 	}
 
 	async createProcess(
@@ -173,15 +174,15 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
 		shouldPersist: boolean
 	): Promise<ITerminalChildProcess> {
 		const executableEnv = await this._shellEnvironmentService.getShellEnv();
-		const id = await this._ptyHostDirectProxy!.createProcess(shellLaunchConfig, cwd, cols, rows, unicodeVersion, env, executableEnv, options, shouldPersist, this._getWorkspaceId(), this._getWorkspaceName());
+		const id = await this._ptyHostDirectProxy.createProcess(shellLaunchConfig, cwd, cols, rows, unicodeVersion, env, executableEnv, options, shouldPersist, this._getWorkspaceId(), this._getWorkspaceName());
 		const pty = this._instantiationService.createInstance(LocalPty, id, shouldPersist);
 		this._ptys.set(id, pty);
 		return pty;
 	}
 
 	async attachToProcess(id: number): Promise<ITerminalChildProcess | undefined> {
 		try {
-			await this._ptyHostDirectProxy!.attachToProcess(id);
+			await this._ptyHostDirectProxy.attachToProcess(id);
 			const pty = this._instantiationService.createInstance(LocalPty, id, true);
 			this._ptys.set(id, pty);
 			return pty;
@@ -193,7 +194,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
 
 	async attachToRevivedProcess(id: number): Promise<ITerminalChildProcess | undefined> {
 		try {
-			const newId = await this._ptyHostDirectProxy!.getRevivedPtyNewId(id) ?? id;
+			const newId = await this._ptyHostDirectProxy.getRevivedPtyNewId(id) ?? id;
 			return await this.attachToProcess(newId);
 		} catch (e) {
 			this._logService.warn(`Couldn't attach to process ${e.message}`);
@@ -202,15 +203,15 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
 	}
 
 	async listProcesses(): Promise<IProcessDetails[]> {
-		return this._ptyHostDirectProxy!.listProcesses();
+		return this._ptyHostDirectProxy.listProcesses();
 	}
 
 	async reduceConnectionGraceTime(): Promise<void> {
-		this._ptyHostDirectProxy!.reduceConnectionGraceTime();
+		this._ptyHostDirectProxy.reduceConnectionGraceTime();
 	}
 
 	async getDefaultSystemShell(osOverride?: OperatingSystem): Promise<string> {
-		return this._ptyHostDirectProxy!.getDefaultSystemShell(osOverride);
+		return this._ptyHostDirectProxy.getDefaultSystemShell(osOverride);
 	}
 
 	async getProfiles(profiles: unknown, defaultProfile: unknown, includeDetectedProfiles?: boolean) {
@@ -219,23 +220,23 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
 	}
 
 	async getEnvironment(): Promise<IProcessEnvironment> {
-		return this._ptyHostDirectProxy!.getEnvironment();
+		return this._ptyHostDirectProxy.getEnvironment();
 	}
 
 	async getShellEnvironment(): Promise<IProcessEnvironment> {
 		return this._shellEnvironmentService.getShellEnv();
 	}
 
 	async getWslPath(original: string, direction: 'unix-to-win' | 'win-to-unix'): Promise<string> {
-		return this._ptyHostDirectProxy!.getWslPath(original, direction);
+		return this._ptyHostDirectProxy.getWslPath(original, direction);
 	}
 
 	async setTerminalLayoutInfo(layoutInfo?: ITerminalsLayoutInfoById): Promise<void> {
 		const args: ISetTerminalLayoutInfoArgs = {
 			workspaceId: this._getWorkspaceId(),
 			tabs: layoutInfo ? layoutInfo.tabs : []
 		};
-		await this._ptyHostDirectProxy!.setTerminalLayoutInfo(args);
+		await this._ptyHostDirectProxy.setTerminalLayoutInfo(args);
 		// Store in the storage service as well to be used when reviving processes as normally this
 		// is stored in memory on the pty host
 		this._storageService.store(TerminalStorageKeys.TerminalLayoutInfo, JSON.stringify(args), StorageScope.WORKSPACE, StorageTarget.MACHINE);