diff --git a/packages/process/src/node/process-manager.ts b/packages/process/src/node/process-manager.ts index 91aad5ed56b89..832979aca869c 100644 --- a/packages/process/src/node/process-manager.ts +++ b/packages/process/src/node/process-manager.ts @@ -42,7 +42,6 @@ export class ProcessManager implements BackendApplicationContribution { register(process: Process): number { const id = this.id; this.processes.set(id, process); - process.onExit(() => this.unregister(process)); process.onError(() => this.unregister(process)); this.id++; return id; @@ -54,7 +53,7 @@ export class ProcessManager implements BackendApplicationContribution { * * @param process the process to unregister from this process manager. */ - protected unregister(process: Process): void { + unregister(process: Process): void { const processLabel = this.getProcessLabel(process); this.logger.debug(`Unregistering process. ${processLabel}`); if (!process.killed) { diff --git a/packages/process/src/node/process.ts b/packages/process/src/node/process.ts index d53b958408f17..b252be6b19428 100644 --- a/packages/process/src/node/process.ts +++ b/packages/process/src/node/process.ts @@ -88,6 +88,8 @@ export abstract class Process { protected readonly closeEmitter: Emitter = new Emitter(); protected readonly errorEmitter: Emitter = new Emitter(); protected _killed = false; + protected _exited = false; + protected _attached = false; /** * The OS process id. @@ -125,6 +127,20 @@ export abstract class Process { return this._killed; } + get exited(): boolean { + return this._exited; + } + set exited(exited: boolean) { + this._exited = exited; + } + + get attached(): boolean { + return this._attached; + } + set attached(attached: boolean) { + this._attached = attached; + } + get onStart(): Event { return this.startEmitter.event; } diff --git a/packages/process/src/node/terminal-process.ts b/packages/process/src/node/terminal-process.ts index 0aeb3c8a2c246..ff99fe63cf851 100644 --- a/packages/process/src/node/terminal-process.ts +++ b/packages/process/src/node/terminal-process.ts @@ -103,8 +103,14 @@ export class TerminalProcess extends Process { // be ignored. if (signal === undefined || signal === 0) { this.emitOnExit(code, undefined); + this.onProcessExited(); + // Unregister process only if terminal already attached, Fix issue #2961. + if (this.attached) { + this.unregisterProcess(); + } } else { this.emitOnExit(undefined, signame(signal)); + this.unregisterProcess(); } process.nextTick(() => { if (signal === undefined || signal === 0) { @@ -168,6 +174,14 @@ export class TerminalProcess extends Process { } } + unregisterProcess(): void { + this.processManager.unregister(this); + } + + onProcessExited(): void { + this.exited = true; + } + resize(cols: number, rows: number): void { this.checkTerminal(); this.terminal!.resize(cols, rows); diff --git a/packages/task/src/browser/quick-open-task.ts b/packages/task/src/browser/quick-open-task.ts index 67f003d83a630..1f94e90b9fee6 100644 --- a/packages/task/src/browser/quick-open-task.ts +++ b/packages/task/src/browser/quick-open-task.ts @@ -215,7 +215,7 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler { if (mode !== QuickOpenMode.OPEN) { return false; } - this.taskService.attach(task.terminalId!, task.taskId); + this.taskService.attach(task.terminalId!, task); return true; } }, diff --git a/packages/task/src/browser/task-service.ts b/packages/task/src/browser/task-service.ts index f44c73210b870..ae2fa3f15de36 100644 --- a/packages/task/src/browser/task-service.ts +++ b/packages/task/src/browser/task-service.ts @@ -998,7 +998,7 @@ export class TaskService implements TaskConfigurationClient { * Reason: Maybe a new task type wants to also be displayed in a terminal. */ if (typeof taskInfo.terminalId === 'number') { - this.attach(taskInfo.terminalId, taskInfo.taskId); + this.attach(taskInfo.terminalId, taskInfo); } return taskInfo; } catch (error) { @@ -1026,11 +1026,11 @@ export class TaskService implements TaskConfigurationClient { const registeredProblemMatchers = this.problemMatcherRegistry.getAll(); items.push(...registeredProblemMatchers.map(matcher => - ({ - label: matcher.label, - value: { problemMatchers: [matcher] }, - description: matcher.name.startsWith('$') ? matcher.name : `$${matcher.name}` - }) + ({ + label: matcher.label, + value: { problemMatchers: [matcher] }, + description: matcher.name.startsWith('$') ? matcher.name : `$${matcher.name}` + }) )); return items; } @@ -1059,11 +1059,7 @@ export class TaskService implements TaskConfigurationClient { terminal.sendText(selectedText); } - async attach(terminalId: number, taskId: number): Promise { - // Get the list of all available running tasks. - const runningTasks: TaskInfo[] = await this.getRunningTasks(); - // Get the corresponding task information based on task id if available. - const taskInfo: TaskInfo | undefined = runningTasks.find((t: TaskInfo) => t.taskId === taskId); + async attach(terminalId: number, taskInfo: TaskInfo): Promise { let widgetOpenMode: WidgetOpenMode = 'open'; if (taskInfo) { const terminalWidget = this.terminalService.getByTerminalId(terminalId); @@ -1079,6 +1075,7 @@ export class TaskService implements TaskConfigurationClient { } } } + const { taskId } = taskInfo; // Create / find a terminal widget to display an execution output of a task that was launched as a command inside a shell. const widget = await this.taskTerminalWidgetManager.open({ created: new Date().toString(), diff --git a/packages/terminal/src/browser/terminal-widget-impl.ts b/packages/terminal/src/browser/terminal-widget-impl.ts index 162fe01deebc2..3a4fa95c4e395 100644 --- a/packages/terminal/src/browser/terminal-widget-impl.ts +++ b/packages/terminal/src/browser/terminal-widget-impl.ts @@ -371,6 +371,7 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget this.connectTerminalProcess(); if (IBaseTerminalServer.validateId(this.terminalId)) { this.onDidOpenEmitter.fire(undefined); + await this.shellTerminalServer.onAttached(this._terminalId); return this.terminalId; } this.onDidOpenFailureEmitter.fire(undefined); diff --git a/packages/terminal/src/common/base-terminal-protocol.ts b/packages/terminal/src/common/base-terminal-protocol.ts index 7a25014a31d05..26c2e7770595d 100644 --- a/packages/terminal/src/common/base-terminal-protocol.ts +++ b/packages/terminal/src/common/base-terminal-protocol.ts @@ -29,6 +29,7 @@ export interface IBaseTerminalServer extends JsonRpcServer getProcessId(id: number): Promise; getProcessInfo(id: number): Promise; getCwdURI(id: number): Promise; + onAttached(id: number): Promise; resize(id: number, cols: number, rows: number): Promise; attach(id: number): Promise; close(id: number): Promise; @@ -171,7 +172,7 @@ export interface MergedEnvironmentVariableCollection { /** * Applies this collection to a process environment. */ - applyToProcessEnvironment(env: { [key: string]: string | null } ): void; + applyToProcessEnvironment(env: { [key: string]: string | null }): void; } export interface SerializableExtensionEnvironmentVariableCollection { diff --git a/packages/terminal/src/node/base-terminal-server.ts b/packages/terminal/src/node/base-terminal-server.ts index 57f31cd139dd6..56eab115d65ed 100644 --- a/packages/terminal/src/node/base-terminal-server.ts +++ b/packages/terminal/src/node/base-terminal-server.ts @@ -87,6 +87,17 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer { }; } + async onAttached(id: number): Promise { + const terminal = this.processManager.get(id); + if (terminal instanceof TerminalProcess) { + // If the process exited before terminal attached, unregister only when finished attach. (Issue #2961) + if (terminal.exited) { + terminal.unregisterProcess(); + } + terminal.attached = true; + } + } + async getCwdURI(id: number): Promise { const terminal = this.processManager.get(id); if (!(terminal instanceof TerminalProcess)) {