From dc189ed8c32ca0edcf10b1a2297319cd9cb503df Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 7 Jul 2023 09:53:58 -0700 Subject: [PATCH] Ensure layout info is fetched from the pty host on reconnects Fixes #187282 --- .../platform/terminal/node/ptyHostService.ts | 5 +- src/vs/platform/terminal/node/ptyService.ts | 5 +- .../terminal/browser/remoteTerminalBackend.ts | 42 ++++++----- .../electron-sandbox/localTerminalBackend.ts | 70 +++++++++---------- 4 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/vs/platform/terminal/node/ptyHostService.ts b/src/vs/platform/terminal/node/ptyHostService.ts index a9b9a2e57ae86..324e4063d95db 100644 --- a/src/vs/platform/terminal/node/ptyHostService.ts +++ b/src/vs/platform/terminal/node/ptyHostService.ts @@ -326,7 +326,10 @@ export class PtyHostService extends Disposable implements IPtyHostService { return this._proxy.setTerminalLayoutInfo(args); } async getTerminalLayoutInfo(args: IGetTerminalLayoutInfoArgs): Promise { - return await this._proxy.getTerminalLayoutInfo(args); + // This is optional as we want reconnect requests to go through only if the pty host exists. + // Revive is handled specially as reviveTerminalProcesses is guaranteed to be called before + // the request for layout info. + return this._optionalProxy?.getTerminalLayoutInfo(args); } async requestDetachInstance(workspaceId: string, instanceId: number): Promise { diff --git a/src/vs/platform/terminal/node/ptyService.ts b/src/vs/platform/terminal/node/ptyService.ts index 4661924d5dcc4..7e017ab5ccdda 100644 --- a/src/vs/platform/terminal/node/ptyService.ts +++ b/src/vs/platform/terminal/node/ptyService.ts @@ -552,8 +552,9 @@ export class PtyService extends Disposable implements IPtyService { }; } catch (e) { this._logService.warn(`Couldn't get layout info, a terminal was probably disconnected`, e.message); - this._logService.info('Reattach to wrong terminal debug info - layout info by id', t); - this._logService.info('Reattach to wrong terminal debug info - _revivePtyIdMap', Array.from(this._revivedPtyIdMap.values())); + this._logService.debug('Reattach to wrong terminal debug info - layout info by id', t); + this._logService.debug('Reattach to wrong terminal debug info - _revivePtyIdMap', Array.from(this._revivedPtyIdMap.values())); + this._logService.debug('Reattach to wrong terminal debug info - _ptys ids', Array.from(this._ptys.keys())); // this will be filtered out and not reconnected return { terminal: null, diff --git a/src/vs/workbench/contrib/terminal/browser/remoteTerminalBackend.ts b/src/vs/workbench/contrib/terminal/browser/remoteTerminalBackend.ts index 855d21f39c9c4..a893c53f6ae46 100644 --- a/src/vs/workbench/contrib/terminal/browser/remoteTerminalBackend.ts +++ b/src/vs/workbench/contrib/terminal/browser/remoteTerminalBackend.ts @@ -337,29 +337,27 @@ class RemoteTerminalBackend extends BaseTerminalBackend implements ITerminalBack // Revive processes if needed const serializedState = this._storageService.get(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE); - const parsed = this._deserializeTerminalState(serializedState); - if (!parsed) { - return undefined; - } - - try { - // Note that remote terminals do not get their environment re-resolved unlike in local terminals - - mark('code/terminal/willReviveTerminalProcessesRemote'); - await this._remoteTerminalChannel.reviveTerminalProcesses(parsed, Intl.DateTimeFormat().resolvedOptions().locale); - mark('code/terminal/didReviveTerminalProcessesRemote'); - this._storageService.remove(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE); - // If reviving processes, send the terminal layout info back to the pty host as it - // will not have been persisted on application exit - const layoutInfo = this._storageService.get(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); - if (layoutInfo) { - mark('code/terminal/willSetTerminalLayoutInfoRemote'); - await this._remoteTerminalChannel.setTerminalLayoutInfo(JSON.parse(layoutInfo)); - mark('code/terminal/didSetTerminalLayoutInfoRemote'); - this._storageService.remove(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); + const reviveBufferState = this._deserializeTerminalState(serializedState); + if (reviveBufferState && reviveBufferState.length > 0) { + try { + // Note that remote terminals do not get their environment re-resolved unlike in local terminals + + mark('code/terminal/willReviveTerminalProcessesRemote'); + await this._remoteTerminalChannel.reviveTerminalProcesses(reviveBufferState, Intl.DateTimeFormat().resolvedOptions().locale); + mark('code/terminal/didReviveTerminalProcessesRemote'); + this._storageService.remove(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE); + // If reviving processes, send the terminal layout info back to the pty host as it + // will not have been persisted on application exit + const layoutInfo = this._storageService.get(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); + if (layoutInfo) { + mark('code/terminal/willSetTerminalLayoutInfoRemote'); + await this._remoteTerminalChannel.setTerminalLayoutInfo(JSON.parse(layoutInfo)); + mark('code/terminal/didSetTerminalLayoutInfoRemote'); + this._storageService.remove(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); + } + } catch (e: unknown) { + this._logService.warn('RemoteTerminalBackend#getTerminalLayoutInfo Error', e && typeof e === 'object' && 'message' in e ? e.message : e); } - } catch (e: unknown) { - this._logService.warn('RemoteTerminalBackend#getTerminalLayoutInfo Error', e && typeof e === 'object' && 'message' in e ? e.message : e); } return this._remoteTerminalChannel.getTerminalLayoutInfo(); diff --git a/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts b/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts index a07bea61575fb..d89cc49042ae2 100644 --- a/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts +++ b/src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts @@ -317,43 +317,41 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke // Revive processes if needed const serializedState = this._storageService.get(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE); - const parsed = this._deserializeTerminalState(serializedState); - if (!parsed) { - return undefined; - } - - try { - // Create variable resolver - const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(); - const lastActiveWorkspace = activeWorkspaceRootUri ? withNullAsUndefined(this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri)) : undefined; - const variableResolver = terminalEnvironment.createVariableResolver(lastActiveWorkspace, await this._terminalProfileResolverService.getEnvironment(this.remoteAuthority), this._configurationResolverService); - - // Re-resolve the environments and replace it on the state so local terminals use a fresh - // environment - mark('code/terminal/willGetReviveEnvironments'); - await Promise.all(parsed.map(state => new Promise(r => { - this._resolveEnvironmentForRevive(variableResolver, state.shellLaunchConfig).then(freshEnv => { - state.processLaunchConfig.env = freshEnv; - r(); - }); - }))); - mark('code/terminal/didGetReviveEnvironments'); - - mark('code/terminal/willReviveTerminalProcesses'); - await this._proxy.reviveTerminalProcesses(workspaceId, parsed, Intl.DateTimeFormat().resolvedOptions().locale); - mark('code/terminal/didReviveTerminalProcesses'); - this._storageService.remove(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE); - // If reviving processes, send the terminal layout info back to the pty host as it - // will not have been persisted on application exit - const layoutInfo = this._storageService.get(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); - if (layoutInfo) { - mark('code/terminal/willSetTerminalLayoutInfo'); - await this._proxy.setTerminalLayoutInfo(JSON.parse(layoutInfo)); - mark('code/terminal/didSetTerminalLayoutInfo'); - this._storageService.remove(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); + const reviveBufferState = this._deserializeTerminalState(serializedState); + if (reviveBufferState && reviveBufferState.length > 0) { + try { + // Create variable resolver + const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(); + const lastActiveWorkspace = activeWorkspaceRootUri ? withNullAsUndefined(this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri)) : undefined; + const variableResolver = terminalEnvironment.createVariableResolver(lastActiveWorkspace, await this._terminalProfileResolverService.getEnvironment(this.remoteAuthority), this._configurationResolverService); + + // Re-resolve the environments and replace it on the state so local terminals use a fresh + // environment + mark('code/terminal/willGetReviveEnvironments'); + await Promise.all(reviveBufferState.map(state => new Promise(r => { + this._resolveEnvironmentForRevive(variableResolver, state.shellLaunchConfig).then(freshEnv => { + state.processLaunchConfig.env = freshEnv; + r(); + }); + }))); + mark('code/terminal/didGetReviveEnvironments'); + + mark('code/terminal/willReviveTerminalProcesses'); + await this._proxy.reviveTerminalProcesses(workspaceId, reviveBufferState, Intl.DateTimeFormat().resolvedOptions().locale); + mark('code/terminal/didReviveTerminalProcesses'); + this._storageService.remove(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE); + // If reviving processes, send the terminal layout info back to the pty host as it + // will not have been persisted on application exit + const layoutInfo = this._storageService.get(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); + if (layoutInfo) { + mark('code/terminal/willSetTerminalLayoutInfo'); + await this._proxy.setTerminalLayoutInfo(JSON.parse(layoutInfo)); + mark('code/terminal/didSetTerminalLayoutInfo'); + this._storageService.remove(TerminalStorageKeys.TerminalLayoutInfo, StorageScope.WORKSPACE); + } + } catch (e: unknown) { + this._logService.warn('LocalTerminalBackend#getTerminalLayoutInfo Error', e && typeof e === 'object' && 'message' in e ? e.message : e); } - } catch (e: unknown) { - this._logService.warn('LocalTerminalBackend#getTerminalLayoutInfo Error', e && typeof e === 'object' && 'message' in e ? e.message : e); } return this._proxy.getTerminalLayoutInfo(layoutArgs);