remove terminal data when terminal is disposed of for reconnected terminals (microsoft#155233)

Author: meganrogge

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

@@ -168,7 +168,7 @@ export interface IPtyHostAttachTarget {
 	fixedDimensions: IFixedTerminalDimensions | undefined;
 	environmentVariableCollections: ISerializableEnvironmentVariableCollections | undefined;
 	reconnectionOwner?: string;
-	task?: { label: string; id: string; lastTask?: string; group?: string };
+	task?: { label: string; id: string; lastTask: string; group?: string };
 }
 
 export enum TitleEventSource {
@@ -469,7 +469,7 @@ export interface IShellLaunchConfig {
 	/**
 	 * This is a terminal that attaches to an already running terminal.
 	 */
-	attachPersistentProcess?: { id: number; findRevivedId?: boolean; pid: number; title: string; titleSource: TitleEventSource; cwd: string; icon?: TerminalIcon; color?: string; hasChildProcesses?: boolean; fixedDimensions?: IFixedTerminalDimensions; environmentVariableCollections?: ISerializableEnvironmentVariableCollections; reconnectionOwner?: string; task?: { label: string; id: string; lastTask?: string; group?: string } };
+	attachPersistentProcess?: { id: number; findRevivedId?: boolean; pid: number; title: string; titleSource: TitleEventSource; cwd: string; icon?: TerminalIcon; color?: string; hasChildProcesses?: boolean; fixedDimensions?: IFixedTerminalDimensions; environmentVariableCollections?: ISerializableEnvironmentVariableCollections; reconnectionOwner?: string; task?: { label: string; id: string; lastTask: string; group?: string } };
 
 	/**
 	 * Whether the terminal process environment should be exactly as provided in
@@ -544,7 +544,7 @@ export interface IShellLaunchConfig {
 	/**
 	 * The task associated with this terminal
 	 */
-	task?: { lastTask?: string; group?: string; label: string; id: string };
+	task?: { lastTask: string; group?: string; label: string; id: string };
 }
 
 export interface ICreateContributedTerminalProfileOptions {

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

@@ -61,7 +61,7 @@ export interface IProcessDetails {
 	fixedDimensions: IFixedTerminalDimensions | undefined;
 	environmentVariableCollections: ISerializableEnvironmentVariableCollections | undefined;
 	reconnectionOwner?: string;
-	task?: { label: string; id: string; lastTask?: string; group?: string };
+	task?: { label: string; id: string; lastTask: string; group?: string };
 }
 
 export type ITerminalTabLayoutInfoDto = IRawTerminalTabLayoutInfo<IProcessDetails>;

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

@@ -200,6 +200,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 
 	private static _nextHandle: number = 0;
 
+	private _tasksReconnected: boolean = false;
 	private _schemaVersion: JsonSchemaVersion | undefined;
 	private _executionEngine: ExecutionEngine | undefined;
 	private _workspaceFolders: IWorkspaceFolder[] | undefined;
@@ -337,11 +338,14 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 			processContext.set(process && !isVirtual);
 		}
 		this._onDidRegisterSupportedExecutions.fire();
+		if (this._jsonTasksSupported && !this._tasksReconnected) {
+			this._reconnectTasks();
+		}
 	}
 
-	private async _restartTasks(): Promise<void> {
+	private async _reconnectTasks(): Promise<void> {
 		const recentlyUsedTasks = await this.readRecentTasks();
-		if (!recentlyUsedTasks) {
+		if (!recentlyUsedTasks.length) {
 			return;
 		}
 		for (const task of recentlyUsedTasks) {
@@ -354,6 +358,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 				this.run(task, undefined, TaskRunSource.Reconnect);
 			}
 		}
+		this._tasksReconnected = true;
 	}
 
 	public get onDidStateChange(): Event<ITaskEvent> {
@@ -618,7 +623,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 		return infosCount > 0;
 	}
 
-	public async registerTaskSystem(key: string, info: ITaskSystemInfo): Promise<void> {
+	public registerTaskSystem(key: string, info: ITaskSystemInfo): void {
 		// Ideally the Web caller of registerRegisterTaskSystem would use the correct key.
 		// However, the caller doesn't know about the workspace folders at the time of the call, even though we know about them here.
 		if (info.platform === Platform.Platform.Web) {
@@ -638,7 +643,6 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 
 		if (this.hasTaskSystemInfo) {
 			this._onDidChangeTaskSystemInfo.fire();
-			await this._restartTasks();
 		}
 	}
 

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

@@ -63,6 +63,8 @@ interface IActiveTerminalData {
 	state?: TaskEventKind;
 }
 
+const ReconnectionType = 'Task';
+
 class InstanceManager {
 	private _currentInstances: number = 0;
 	private _counter: number = 0;
@@ -255,6 +257,7 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 				this._logService.trace(`Could not reconnect to terminal ${terminal.instanceId} with process details ${terminal.shellLaunchConfig.attachPersistentProcess}`);
 			}
 		}
+		this._hasReconnected = true;
 	}
 
 	public get onDidStateChange(): Event<ITaskEvent> {
@@ -270,10 +273,13 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 	}
 
 	public reconnect(task: Task, resolver: ITaskResolver, trigger: string = Triggers.command): ITaskExecuteResult | undefined {
-		const terminals = this._terminalService.getReconnectedTerminals('Task');
+		const terminals = this._terminalService.getReconnectedTerminals(ReconnectionType);
+		if (!terminals || terminals?.length === 0) {
+			return;
+		}
 		if (!this._hasReconnected && terminals && terminals.length > 0) {
+			this._reviveTerminals();
 			this._reconnectToTerminals(terminals);
-			this._hasReconnected = true;
 		}
 		if (this._tasksToReconnect.includes(task._id)) {
 			this._lastTask = new VerifiedTask(task, resolver, trigger);
@@ -449,12 +455,14 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 		}
 	}
 
-	private _removeFromActiveTasks(task: Task): void {
-		if (!this._activeTasks[task.getMapKey()]) {
+	private _removeFromActiveTasks(task: Task | string): void {
+		const key = typeof task === 'string' ? task : task.getMapKey();
+		if (!this._activeTasks[key]) {
 			return;
 		}
-		delete this._activeTasks[task.getMapKey()];
-		this._removeInstances(task);
+		const taskToRemove = this._activeTasks[key];
+		delete this._activeTasks[key];
+		this._removeInstances(taskToRemove.task);
 	}
 
 	private _fireTaskEvent(event: ITaskEvent) {
@@ -1059,7 +1067,7 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 		const isShellCommand = task.command.runtime === RuntimeType.Shell;
 		const needsFolderQualification = this._contextService.getWorkbenchState() === WorkbenchState.WORKSPACE;
 		const terminalName = this._createTerminalName(task);
-		const type = 'Task';
+		const type = ReconnectionType;
 		const originalCommand = task.command.name;
 		if (isShellCommand) {
 			let os: Platform.OperatingSystem;
@@ -1289,17 +1297,40 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 	}
 
 	private _reviveTerminals(): void {
-		if (Object.entries(this._terminals).length === 0) {
-			for (const terminal of this._terminalService.instances) {
-				if (terminal.shellLaunchConfig.attachPersistentProcess?.task?.lastTask) {
-					this._terminals[terminal.instanceId] = { lastTask: terminal.shellLaunchConfig.attachPersistentProcess.task.lastTask, group: terminal.shellLaunchConfig.attachPersistentProcess.task.group, terminal };
-				}
+		if (Object.entries(this._terminals).length > 0) {
+			return;
+		}
+		const terminals = this._terminalService.getReconnectedTerminals(ReconnectionType)?.filter(t => !t.isDisposed);
+		if (!terminals?.length) {
+			return;
+		}
+		for (const terminal of terminals) {
+			const task = terminal.shellLaunchConfig.attachPersistentProcess?.task;
+			if (!task) {
+				continue;
 			}
+			const terminalData = { lastTask: task.lastTask, group: task.group, terminal };
+			this._terminals[terminal.instanceId] = terminalData;
+			terminal.onDisposed(() => this._deleteTaskAndTerminal(terminal, terminalData));
+		}
+	}
+
+	private _deleteTaskAndTerminal(terminal: ITerminalInstance, terminalData: ITerminalData): void {
+		delete this._terminals[terminal.instanceId];
+		delete this._sameTaskTerminals[terminalData.lastTask];
+		this._idleTaskTerminals.delete(terminalData.lastTask);
+		// Delete the task now as a work around for cases when the onExit isn't fired.
+		// This can happen if the terminal wasn't shutdown with an "immediate" flag and is expected.
+		// For correct terminal re-use, the task needs to be deleted immediately.
+		// Note that this shouldn't be a problem anymore since user initiated terminal kills are now immediate.
+		const mapKey = terminalData.lastTask;
+		this._removeFromActiveTasks(mapKey);
+		if (this._busyTasks[mapKey]) {
+			delete this._busyTasks[mapKey];
 		}
 	}
 
 	private async _createTerminal(task: CustomTask | ContributedTask, resolver: VariableResolver, workspaceFolder: IWorkspaceFolder | undefined): Promise<[ITerminalInstance | undefined, TaskError | undefined]> {
-		this._reviveTerminals();
 		const platform = resolver.taskSystemInfo ? resolver.taskSystemInfo.platform : Platform.platform;
 		const options = await this._resolveOptions(resolver, task.command.options);
 		const presentationOptions = task.command.presentation;
@@ -1398,29 +1429,14 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
 		}
 
 		this._terminalCreationQueue = this._terminalCreationQueue.then(() => this._doCreateTerminal(group, launchConfigs!));
-		const result: ITerminalInstance = (await this._terminalCreationQueue)!;
-		result.shellLaunchConfig.task = { lastTask: taskKey, group, label: task._label, id: task._id };
-		result.shellLaunchConfig.reconnectionOwner = 'Task';
-		const terminalKey = result.instanceId.toString();
-		result.onDisposed(() => {
-			const terminalData = this._terminals[terminalKey];
-			if (terminalData) {
-				delete this._terminals[terminalKey];
-				delete this._sameTaskTerminals[terminalData.lastTask];
-				this._idleTaskTerminals.delete(terminalData.lastTask);
-				// Delete the task now as a work around for cases when the onExit isn't fired.
-				// This can happen if the terminal wasn't shutdown with an "immediate" flag and is expected.
-				// For correct terminal re-use, the task needs to be deleted immediately.
-				// Note that this shouldn't be a problem anymore since user initiated terminal kills are now immediate.
-				const mapKey = task.getMapKey();
-				this._removeFromActiveTasks(task);
-				if (this._busyTasks[mapKey]) {
-					delete this._busyTasks[mapKey];
-				}
-			}
-		});
-		this._terminals[terminalKey] = { terminal: result, lastTask: taskKey, group };
-		return [result, undefined];
+		const terminal: ITerminalInstance = (await this._terminalCreationQueue)!;
+		terminal.shellLaunchConfig.task = { lastTask: taskKey, group, label: task._label, id: task._id };
+		terminal.shellLaunchConfig.reconnectionOwner = ReconnectionType;
+		const terminalKey = terminal.instanceId.toString();
+		const terminalData = { terminal: terminal, lastTask: taskKey, group };
+		terminal.onDisposed(() => this._deleteTaskAndTerminal(terminal, terminalData));
+		this._terminals[terminalKey] = terminalData;
+		return [terminal, undefined];
 	}
 
 	private _buildShellCommandLine(platform: Platform.Platform, shellExecutable: string, shellOptions: IShellConfiguration | undefined, command: CommandString, originalCommand: CommandString | undefined, args: CommandString[]): string {