From 3c1bf666378007f47e77c57361b52af075d9c950 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 16 May 2023 09:52:59 -0700 Subject: [PATCH] Use getDelayedChannel to avoid awaiting promise Part of #175335 --- .../electron-sandbox/localTerminalBackend.ts | 80 +++++++------------ 1 file changed, 31 insertions(+), 49 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts b/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts index 221062314e3a0..04533638740e2 100644 --- a/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts +++ b/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts @@ -33,7 +33,7 @@ import { getWorkspaceForTerminal } from 'vs/workbench/services/configurationReso import { INativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-sandbox/environmentService'; import { Client as MessagePortClient } from 'vs/base/parts/ipc/common/ipc.mp'; import { acquirePort } from 'vs/base/parts/ipc/electron-sandbox/ipc.mp'; -import { ProxyChannel } from 'vs/base/parts/ipc/common/ipc'; +import { getDelayedChannel, ProxyChannel } from 'vs/base/parts/ipc/common/ipc'; import { mark } from 'vs/base/common/performance'; import { ILifecycleService, LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { DeferredPromise } from 'vs/base/common/async'; @@ -52,10 +52,8 @@ export class LocalTerminalBackendContribution implements IWorkbenchContribution class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBackend { readonly remoteAuthority = undefined; - // HACK: ProxyChannels behave strangely when interacting with promises and seemingly never - // resolve, so _proxyReady is awaited before every proxy use - private _proxy?: IPtyService; - private readonly _proxyReady: DeferredPromise; + private readonly _proxy: IPtyService; + private readonly _clientEventually: DeferredPromise = new DeferredPromise(); private readonly _ptys: Map = new Map(); @@ -83,7 +81,8 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke ) { super(_localPtyService, logService, notificationService, historyService, _configurationResolverService, workspaceContextService); - this._proxyReady = new DeferredPromise(); + this._proxy = ProxyChannel.toService(getDelayedChannel(this._clientEventually.p.then(client => client.getChannel(TerminalIpcChannels.PtyHostWindow)))); + this._connectToDirectProxy(); } @@ -101,24 +100,22 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke // 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}`); - const proxy = ProxyChannel.toService(client.getChannel(TerminalIpcChannels.PtyHostWindow)); - this._proxy = proxy; - this._proxyReady.complete(); + this._clientEventually.complete(client); // Attach process listeners - proxy.onProcessData(e => this._ptys.get(e.id)?.handleData(e.event)); - proxy.onDidChangeProperty(e => this._ptys.get(e.id)?.handleDidChangeProperty(e.property)); - proxy.onProcessExit(e => { + this._proxy.onProcessData(e => this._ptys.get(e.id)?.handleData(e.event)); + this._proxy.onDidChangeProperty(e => this._ptys.get(e.id)?.handleDidChangeProperty(e.property)); + this._proxy.onProcessExit(e => { const pty = this._ptys.get(e.id); if (pty) { pty.handleExit(e.event); this._ptys.delete(e.id); } }); - proxy.onProcessReady(e => this._ptys.get(e.id)?.handleReady(e.event)); - proxy.onProcessReplay(e => this._ptys.get(e.id)?.handleReplay(e.event)); - proxy.onProcessOrphanQuestion(e => this._ptys.get(e.id)?.handleOrphanQuestion()); - proxy.onDidRequestDetach(e => this._onDidRequestDetach.fire(e)); + this._proxy.onProcessReady(e => this._ptys.get(e.id)?.handleReady(e.event)); + this._proxy.onProcessReplay(e => this._ptys.get(e.id)?.handleReplay(e.event)); + this._proxy.onProcessOrphanQuestion(e => this._ptys.get(e.id)?.handleOrphanQuestion()); + this._proxy.onDidRequestDetach(e => this._onDidRequestDetach.fire(e)); // Listen for config changes const initialConfig = this._configurationService.getValue(TERMINAL_CONFIG_SECTION); @@ -126,19 +123,19 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke // Ensure the reply is value const reply = initialConfig.autoReplies[match] as string | null; if (reply) { - proxy.installAutoReply(match, reply); + this._proxy.installAutoReply(match, reply); } } // TODO: Could simplify update to a single call this._register(this._configurationService.onDidChangeConfiguration(async e => { if (e.affectsConfiguration(TerminalSettingId.AutoReplies)) { - proxy.uninstallAllAutoReplies(); + this._proxy.uninstallAllAutoReplies(); const config = this._configurationService.getValue(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) { - proxy.installAutoReply(match, reply); + this._proxy.installAutoReply(match, reply); } } } @@ -147,8 +144,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke } async requestDetachInstance(workspaceId: string, instanceId: number): Promise { - await this._proxyReady.p; - return this._proxy!.requestDetachInstance(workspaceId, instanceId); + return this._proxy.requestDetachInstance(workspaceId, instanceId); } async acceptDetachInstanceReply(requestId: number, persistentProcessId?: number): Promise { @@ -156,30 +152,25 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke this._logService.warn('Cannot attach to feature terminals, custom pty terminals, or those without a persistentProcessId'); return; } - await this._proxyReady.p; - return this._proxy!.acceptDetachInstanceReply(requestId, persistentProcessId); + return this._proxy.acceptDetachInstanceReply(requestId, persistentProcessId); } async persistTerminalState(): Promise { const ids = Array.from(this._ptys.keys()); - await this._proxyReady.p; - const serialized = await this._proxy!.serializeTerminalState(ids); + const serialized = await this._proxy.serializeTerminalState(ids); this._storageService.store(TerminalStorageKeys.TerminalBufferState, serialized, StorageScope.WORKSPACE, StorageTarget.MACHINE); } async updateTitle(id: number, title: string, titleSource: TitleEventSource): Promise { - await this._proxyReady.p; - await this._proxy!.updateTitle(id, title, titleSource); + await this._proxy.updateTitle(id, title, titleSource); } async updateIcon(id: number, userInitiated: boolean, icon: URI | { light: URI; dark: URI } | { id: string; color?: { id: string } }, color?: string): Promise { - await this._proxyReady.p; - await this._proxy!.updateIcon(id, userInitiated, icon, color); + await this._proxy.updateIcon(id, userInitiated, icon, color); } async updateProperty(id: number, property: ProcessPropertyType, value: IProcessPropertyMap[T]): Promise { - await this._proxyReady.p; - return this._proxy!.updateProperty(id, property, value); + return this._proxy.updateProperty(id, property, value); } async createProcess( @@ -193,8 +184,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke shouldPersist: boolean ): Promise { const executableEnv = await this._shellEnvironmentService.getShellEnv(); - await this._proxyReady.p; - const id = await this._proxy!.createProcess(shellLaunchConfig, cwd, cols, rows, unicodeVersion, env, executableEnv, options, shouldPersist, this._getWorkspaceId(), this._getWorkspaceName()); + const id = await this._proxy.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; @@ -202,8 +192,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke async attachToProcess(id: number): Promise { try { - await this._proxyReady.p; - await this._proxy!.attachToProcess(id); + await this._proxy.attachToProcess(id); const pty = this._instantiationService.createInstance(LocalPty, id, true); this._ptys.set(id, pty); return pty; @@ -215,8 +204,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke async attachToRevivedProcess(id: number): Promise { try { - await this._proxyReady.p; - const newId = await this._proxy!.getRevivedPtyNewId(id) ?? id; + const newId = await this._proxy.getRevivedPtyNewId(id) ?? id; return await this.attachToProcess(newId); } catch (e) { this._logService.warn(`Couldn't attach to process ${e.message}`); @@ -225,18 +213,15 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke } async listProcesses(): Promise { - await this._proxyReady.p; - return this._proxy!.listProcesses(); + return this._proxy.listProcesses(); } async reduceConnectionGraceTime(): Promise { - await this._proxyReady.p; - this._proxy!.reduceConnectionGraceTime(); + this._proxy.reduceConnectionGraceTime(); } async getDefaultSystemShell(osOverride?: OperatingSystem): Promise { - await this._proxyReady; - return this._proxy!.getDefaultSystemShell(osOverride); + return this._proxy.getDefaultSystemShell(osOverride); } async getProfiles(profiles: unknown, defaultProfile: unknown, includeDetectedProfiles?: boolean) { @@ -245,8 +230,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke } async getEnvironment(): Promise { - await this._proxyReady.p; - return this._proxy!.getEnvironment(); + return this._proxy.getEnvironment(); } async getShellEnvironment(): Promise { @@ -254,8 +238,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke } async getWslPath(original: string, direction: 'unix-to-win' | 'win-to-unix'): Promise { - await this._proxyReady.p; - return this._proxy!.getWslPath(original, direction); + return this._proxy.getWslPath(original, direction); } async setTerminalLayoutInfo(layoutInfo?: ITerminalsLayoutInfoById): Promise { @@ -263,8 +246,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke workspaceId: this._getWorkspaceId(), tabs: layoutInfo ? layoutInfo.tabs : [] }; - await this._proxyReady.p; - await this._proxy!.setTerminalLayoutInfo(args); + await this._proxy.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);