Prevent terminals being attached to by multiple windows

I was no able to reproduce the issue where users were seeing a terminal get reconnected
to from multiple windows. This is a defensive change that does the following:

- When a persistent process is being attached to, check if it is disconnected. If so,
confirm that it is orphaned, continue if so and throw if not.
- Silently recreate a new process with warning logs if an attach fails.

Fixes #133542
This commit is contained in:
Daniel Imms
2022-06-16 17:12:00 -07:00
parent cb18a4870c
commit cced28f080
2 changed files with 23 additions and 10 deletions
+11 -2
View File
@@ -210,10 +210,11 @@ export class PtyService extends Disposable implements IPtyService {
async attachToProcess(id: number): Promise<void> {
try {
this._throwIfNoPty(id).attach();
await this._throwIfNoPty(id).attach();
this._logService.trace(`Persistent process reconnection "${id}"`);
} catch (e) {
this._logService.trace(`Persistent process reconnection "${id}" failed`, e.message);
throw e;
}
}
@@ -543,8 +544,16 @@ export class PersistentTerminalProcess extends Disposable {
}));
}
attach(): void {
async attach(): Promise<void> {
this._logService.trace('persistentTerminalProcess#attach', this._persistentProcessId);
if (!this._disconnectRunner1.isScheduled() && !this._disconnectRunner2.isScheduled()) {
// Something wrong happened if the disconnect runner is not canceled, this likely means
// multiple windows attempted to attach.
if (!await this._isOrphaned()) {
throw new Error(`Cannot attach to persistent process "${this._persistentProcessId}", it is already adopted`);
}
this._logService.warn(`Persistent process "${this._persistentProcessId}": Process had no disconnect runners but was an orphan`);
}
this._disconnectRunner1.cancel();
this._disconnectRunner2.cancel();
}
@@ -210,7 +210,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
this._dimensions.rows = rows;
this._isScreenReaderModeEnabled = isScreenReaderModeEnabled;
let newProcess: ITerminalChildProcess;
let newProcess: ITerminalChildProcess | undefined;
if (shellLaunchConfig.customPtyImplementation) {
this._processType = ProcessType.PsuedoTerminal;
@@ -251,10 +251,12 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
if (result) {
newProcess = result;
} else {
this._logService.trace(`Attach to process failed for terminal ${shellLaunchConfig.attachPersistentProcess}`);
return undefined;
// Warn and just create a new terminal if attach failed for some reason
this._logService.warn(`Attach to process failed for terminal`, shellLaunchConfig.attachPersistentProcess);
shellLaunchConfig.attachPersistentProcess = undefined;
}
} else {
}
if (!newProcess) {
await this._terminalProfileResolverService.resolveShellLaunchConfig(shellLaunchConfig, {
remoteAuthority: this.remoteAuthority,
os: this.os
@@ -294,10 +296,12 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
if (result) {
newProcess = result;
} else {
this._logService.trace(`Attach to process failed for terminal ${shellLaunchConfig.attachPersistentProcess}`);
return undefined;
// Warn and just create a new terminal if attach failed for some reason
this._logService.warn(`Attach to process failed for terminal`, shellLaunchConfig.attachPersistentProcess);
shellLaunchConfig.attachPersistentProcess = undefined;
}
} else {
}
if (!newProcess) {
newProcess = await this._launchLocalProcess(backend, shellLaunchConfig, cols, rows, this.userHome, isScreenReaderModeEnabled, variableResolver);
}
if (!this._isDisposed) {
@@ -335,7 +339,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
if (this._preLaunchInputQueue.length > 0 && this._process) {
// Send any queued data that's waiting
newProcess.input(this._preLaunchInputQueue.join(''));
newProcess!.input(this._preLaunchInputQueue.join(''));
this._preLaunchInputQueue.length = 0;
}
}),