wait for terminal reconnection to try to reconnect tasks (microsoft#158391)

Author: meganrogge

src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts

@@ -8,7 +8,7 @@ import { IStringDictionary } from 'vs/base/common/collections';
 import { Emitter, Event } from 'vs/base/common/event';
 import * as glob from 'vs/base/common/glob';
 import * as json from 'vs/base/common/json';
-import { Disposable, IDisposable, IReference } from 'vs/base/common/lifecycle';
+import { Disposable, dispose, IDisposable, IReference } from 'vs/base/common/lifecycle';
 import { LRUCache, Touch } from 'vs/base/common/map';
 import * as Objects from 'vs/base/common/objects';
 import { ValidationState, ValidationStatus } from 'vs/base/common/parsers';
@@ -53,7 +53,7 @@ import { ITaskExecuteResult, ITaskResolver, ITaskSummary, ITaskSystem, ITaskSyst
 import { getTemplates as getTaskTemplates } from 'vs/workbench/contrib/tasks/common/taskTemplates';
 
 import * as TaskConfig from '../common/taskConfiguration';
-import { TerminalTaskSystem } from './terminalTaskSystem';
+import { terminalsNotReconnectedExitCode, TerminalTaskSystem } from './terminalTaskSystem';
 
 import { IQuickInputService, IQuickPick, IQuickPickItem, IQuickPickSeparator, QuickPickInput } from 'vs/platform/quickinput/common/quickInput';
 
@@ -181,6 +181,11 @@ class TaskMap {
 	}
 }
 
+interface EventBarrier {
+	isOpen: boolean;
+	queuedEvent?: boolean;
+}
+
 export abstract class AbstractTaskService extends Disposable implements ITaskService {
 
 	// private static autoDetectTelemetryName: string = 'taskServer.autoDetect';
@@ -195,6 +200,8 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 
 	private static _nextHandle: number = 0;
 
+	private _reconnectionBarrier: EventBarrier = { isOpen: true };
+
 	private _tasksReconnected: boolean = false;
 	private _schemaVersion: JsonSchemaVersion | undefined;
 	private _executionEngine: ExecutionEngine | undefined;
@@ -209,7 +216,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 	protected _workspaceTasksPromise?: Promise<Map<string, IWorkspaceFolderTaskResult>>;
 
 	protected _taskSystem?: ITaskSystem;
-	protected _taskSystemListener?: IDisposable;
+	protected _taskSystemListeners?: IDisposable[] = [];
 	private _recentlyUsedTasksV1: LRUCache<string, string> | undefined;
 	private _recentlyUsedTasks: LRUCache<string, string> | undefined;
 
@@ -219,6 +226,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 
 	protected _outputChannel: IOutputChannel;
 	protected readonly _onDidStateChange: Emitter<ITaskEvent>;
+	protected readonly _onDidReconnectToTerminals: Emitter<void> = new Emitter();
 	private _waitForSupportedExecutions: Promise<void>;
 	private _onDidRegisterSupportedExecutions: Emitter<void> = new Emitter();
 	private _onDidChangeTaskSystemInfo: Emitter<void> = new Emitter();
@@ -265,7 +273,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 
 		this._workspaceTasksPromise = undefined;
 		this._taskSystem = undefined;
-		this._taskSystemListener = undefined;
+		this._taskSystemListeners = undefined;
 		this._outputChannel = this._outputService.getChannel(AbstractTaskService.OutputChannelId)!;
 		this._providers = new Map<number, ITaskProvider>();
 		this._providerTypes = new Map<number, string>();
@@ -283,6 +291,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 			if (!this._taskSystem && !this._workspaceTasksPromise) {
 				return;
 			}
+
 			if (!this._taskSystem || this._taskSystem instanceof TerminalTaskSystem) {
 				this._outputChannel.clear();
 			}
@@ -328,6 +337,8 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 		this._waitForSupportedExecutions = new Promise(resolve => {
 			once(this._onDidRegisterSupportedExecutions.event)(() => resolve());
 		});
+		this._register(this.onDidReconnectToTerminals(async () => await this._attemptTaskReconnection()));
+		this._register(this._onDidRegisterSupportedExecutions.event(async () => await this._attemptTaskReconnection()));
 		this._upgrade();
 	}
 
@@ -346,43 +357,65 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 			processContext.set(process && !isVirtual);
 		}
 		this._onDidRegisterSupportedExecutions.fire();
-		if (this._configurationService.getValue(TaskSettingId.Reconnection) === true && this._jsonTasksSupported && !this._tasksReconnected) {
-			this._reconnectTasks();
+	}
+
+	private async _attemptTaskReconnection(): Promise<void> {
+		if (!this._reconnectionBarrier.isOpen) {
+			this._reconnectionBarrier.queuedEvent = true;
+			return;
+		}
+		this._reconnectionBarrier.isOpen = false;
+		if (!this._taskSystem) {
+			this._logService.info('getting task system before reconnection');
+			await this._getTaskSystem();
+		}
+		this._logService.info(`attempting task reconnection, jsonTasksSupported: ${this._jsonTasksSupported}, reconnection pending ${!this._tasksReconnected}`);
+		if (this._configurationService.getValue(TaskSettingId.Reconnection) === true && !this._tasksReconnected) {
+			this._tasksReconnected = await this._reconnectTasks();
+		}
+		this._reconnectionBarrier.isOpen = true;
+		if (this._reconnectionBarrier.queuedEvent) {
+			this._reconnectionBarrier.queuedEvent = undefined;
+			await this._attemptTaskReconnection();
 		}
 	}
 
-	private async _reconnectTasks(): Promise<void> {
+	private async _reconnectTasks(): Promise<boolean> {
 		if (this._lifecycleService.startupKind !== StartupKind.ReloadedWindow) {
 			this._tasksReconnected = true;
 			this._storageService.remove(AbstractTaskService.PersistentTasks_Key, StorageScope.WORKSPACE);
-			return;
+			return true;
 		}
 		const tasks = await this.getSavedTasks('persistent');
-		if (!this._taskSystem) {
-			await this._getTaskSystem();
-		}
 		if (!tasks.length) {
-			this._tasksReconnected = true;
-			return;
+			return true;
 		}
-
 		for (const task of tasks) {
+			let result;
 			if (ConfiguringTask.is(task)) {
 				const resolved = await this.tryResolveTask(task);
 				if (resolved) {
-					this.run(resolved, undefined, TaskRunSource.Reconnect);
+					result = await this.run(resolved, undefined, TaskRunSource.Reconnect);
 				}
 			} else {
-				this.run(task, undefined, TaskRunSource.Reconnect);
+				result = await this.run(task, undefined, TaskRunSource.Reconnect);
+			}
+			if (result?.exitCode === terminalsNotReconnectedExitCode) {
+				this._logService.trace('Terminals were not reconnected');
+				return false;
 			}
 		}
-		this._tasksReconnected = true;
+		return true;
 	}
 
 	public get onDidStateChange(): Event<ITaskEvent> {
 		return this._onDidStateChange.event;
 	}
 
+	public get onDidReconnectToTerminals(): Event<void> {
+		return this._onDidReconnectToTerminals.event;
+	}
+
 	public get supportsMultipleTaskExecutions(): boolean {
 		return this.inTerminal();
 	}
@@ -607,7 +640,10 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 	}
 
 	protected _disposeTaskSystemListeners(): void {
-		this._taskSystemListener?.dispose();
+		if (this._taskSystemListeners) {
+			dispose(this._taskSystemListeners);
+			this._taskSystemListeners = undefined;
+		}
 	}
 
 	public registerTaskProvider(provider: ITaskProvider, type: string): IDisposable {

src/vs/workbench/contrib/tasks/browser/taskService.ts

@@ -23,13 +23,20 @@ export class TaskService extends AbstractTaskService {
 		} else {
 			throw new Error(TaskService.ProcessTaskSystemSupportMessage);
 		}
-		this._taskSystemListener = this._taskSystem!.onDidStateChange((event) => {
-			if (this._taskSystem) {
-				this._taskRunningState.set(this._taskSystem.isActiveSync());
-			}
-			this._onDidStateChange.fire(event);
-		});
-		return this._taskSystem!;
+		const taskSystem = this._createTerminalTaskSystem();
+		this._taskSystem = taskSystem;
+		this._taskSystemListeners =
+			[
+				taskSystem.onDidStateChange((event) => {
+					this._taskRunningState.set(taskSystem.isActiveSync());
+					this._onDidStateChange.fire(event);
+				}),
+				taskSystem.onDidReconnectToTerminals((event) => {
+					this._taskRunningState.set(taskSystem.isActiveSync());
+					this._onDidReconnectToTerminals.fire(event);
+				})
+			];
+		return this._taskSystem;
 	}
 
 	protected _computeLegacyConfiguration(workspaceFolder: IWorkspaceFolder): Promise<IWorkspaceFolderConfigurationResult> {

src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts

@@ -28,7 +28,7 @@ import { Codicon } from 'vs/base/common/codicons';
 import { Schemas } from 'vs/base/common/network';
 import { URI } from 'vs/base/common/uri';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import { ILogService } from 'vs/platform/log/common/log';
+import { ILogService, LogLevel } from 'vs/platform/log/common/log';
 import { INotificationService } from 'vs/platform/notification/common/notification';
 import { IShellLaunchConfig, TerminalLocation, TerminalSettingId, WaitOnExitValue } from 'vs/platform/terminal/common/terminal';
 import { formatMessageForTerminal } from 'vs/platform/terminal/common/terminalStrings';
@@ -119,6 +119,8 @@ class VariableResolver {
 	}
 }
 
+export const terminalsNotReconnectedExitCode = 7777;
+
 export class VerifiedTask {
 	readonly task: Task;
 	readonly resolver: ITaskResolver;
@@ -210,6 +212,7 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 	private _terminalCreationQueue: Promise<ITerminalInstance | void> = Promise.resolve();
 	private _hasReconnected: boolean = false;
 	private readonly _onDidStateChange: Emitter<ITaskEvent>;
+	private readonly _onDidReconnectToTerminals: Emitter<void> = new Emitter();
 	private _reconnectedTerminals: ITerminalInstance[] | undefined;
 
 	get taskShellIntegrationStartSequence(): string {
@@ -252,15 +255,17 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 		this._onDidStateChange = new Emitter();
 		this._taskSystemInfoResolver = taskSystemInfoResolver;
 		this._register(this._terminalStatusManager = new TaskTerminalStatus(taskService));
-		// connection state changes before this is created sometimes
-		this._reconnectToTerminals();
 		this._register(this._terminalService.onDidChangeConnectionState(() => this._reconnectToTerminals()));
 	}
 
 	public get onDidStateChange(): Event<ITaskEvent> {
 		return this._onDidStateChange.event;
 	}
 
+	public get onDidReconnectToTerminals(): Event<void> {
+		return this._onDidReconnectToTerminals.event;
+	}
+
 	private _log(value: string): void {
 		this._appendOutput(value + '\n');
 	}
@@ -270,8 +275,15 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 	}
 
 	public reconnect(task: Task, resolver: ITaskResolver): ITaskExecuteResult {
-		if (!this._reconnectedTerminals?.length) {
-			throw new Error('Persistent tasks were not updated correctly - no terminals for reconnection');
+		if (!this._reconnectedTerminals) {
+			// terminalService.onDidChangeConnectionState might have already fired
+			// before this gets created
+			this._logService.trace('Reconnecting to terminals before running');
+			this._reconnectToTerminals();
+			if (!this._reconnectedTerminals) {
+				this._logService.trace('Returning, terminals have not been reconnected yet');
+				return { kind: TaskExecuteKind.Started, promise: Promise.resolve({ exitCode: terminalsNotReconnectedExitCode }), task } as ITaskExecuteResult;
+			}
 		}
 		return this.run(task, resolver, Triggers.reconnect);
 	}
@@ -1327,21 +1339,32 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 
 	private _reconnectToTerminals(): void {
 		if (this._hasReconnected) {
+			this._logService.trace(`Already reconnected, to ${this._reconnectedTerminals?.length} terminals so returning`);
 			return;
 		}
 		this._reconnectedTerminals = this._terminalService.getReconnectedTerminals(ReconnectionType)?.filter(t => !t.isDisposed);
-		if (!this._reconnectedTerminals?.length) {
+		this._logService.trace(`Attempting reconnection of ${this._reconnectedTerminals?.length} terminals`);
+		if (!this._reconnectedTerminals) {
+			this._hasReconnected = false;
+		} else if (!this._reconnectedTerminals?.length) {
+			this._logService.trace(`No terminals to reconnect to so returning`);
+			this._hasReconnected = true;
 			return;
-		}
-		for (const terminal of this._reconnectedTerminals) {
-			const task = terminal.shellLaunchConfig.attachPersistentProcess?.reconnectionProperties?.data as IReconnectionTaskData;
-			if (!task) {
-				continue;
+		} else {
+			for (const terminal of this._reconnectedTerminals) {
+				const task = terminal.shellLaunchConfig.attachPersistentProcess?.reconnectionProperties?.data as IReconnectionTaskData;
+				if (this._logService.getLevel() <= LogLevel.Trace) {
+					this._logService.trace(`Reconnecting to task: ${JSON.stringify(task)}`);
+				}
+				if (!task) {
+					continue;
+				}
+				const terminalData = { lastTask: task.lastTask, group: task.group, terminal };
+				this._terminals[terminal.instanceId] = terminalData;
 			}
-			const terminalData = { lastTask: task.lastTask, group: task.group, terminal };
-			this._terminals[terminal.instanceId] = terminalData;
+			this._hasReconnected = true;
+			this._onDidReconnectToTerminals.fire();
 		}
-		this._hasReconnected = true;
 	}
 
 	private _deleteTaskAndTerminal(terminal: ITerminalInstance, terminalData: ITerminalData): void {

src/vs/workbench/contrib/tasks/common/taskSystem.ts

@@ -102,6 +102,7 @@ export interface ITaskSystemInfoResolver {
 
 export interface ITaskSystem {
 	onDidStateChange: Event<ITaskEvent>;
+	onDidReconnectToTerminals: Event<void>;
 	reconnect(task: Task, resolver: ITaskResolver): ITaskExecuteResult;
 	run(task: Task, resolver: ITaskResolver): ITaskExecuteResult;
 	rerun(): ITaskExecuteResult | undefined;

src/vs/workbench/contrib/tasks/electron-sandbox/taskService.ts

@@ -131,13 +131,19 @@ export class TaskService extends AbstractTaskService {
 		if (this._taskSystem) {
 			return this._taskSystem;
 		}
-		this._taskSystem = this._createTerminalTaskSystem();
-		this._taskSystemListener = this._taskSystem!.onDidStateChange((event) => {
-			if (this._taskSystem) {
-				this._taskRunningState.set(this._taskSystem.isActiveSync());
-			}
-			this._onDidStateChange.fire(event);
-		});
+		const taskSystem = this._createTerminalTaskSystem();
+		this._taskSystem = taskSystem;
+		this._taskSystemListeners =
+			[
+				this._taskSystem.onDidStateChange((event) => {
+					this._taskRunningState.set(this._taskSystem!.isActiveSync());
+					this._onDidStateChange.fire(event);
+				}),
+				this._taskSystem.onDidReconnectToTerminals((event) => {
+					this._taskRunningState.set(this._taskSystem!.isActiveSync());
+					this._onDidReconnectToTerminals.fire(event);
+				})
+			];
 		return this._taskSystem;
 	}
 

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

@@ -383,6 +383,7 @@ export class TerminalService implements ITerminalService {
 	private _setConnected() {
 		this._connectionState = TerminalConnectionState.Connected;
 		this._onDidChangeConnectionState.fire();
+		this._logService.trace('Reconnected to terminals');
 	}
 
 	private async _reconnectToRemoteTerminals(): Promise<void> {