Fix eclipse-theia#2961: Output of short-lived tasks is not shown

Signed-off-by: Esther Perelman <[email protected]>
EstherPerelman · Apr 28, 2021 · 6f53b72 · 6f53b72
1 parent a8dec8d
commit 6f53b72
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 15 deletions.
diff --git 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
@@ -88,6 +88,8 @@ export abstract class Process {
     protected readonly closeEmitter: Emitter<IProcessExitEvent> = new Emitter<IProcessExitEvent>();
     protected readonly errorEmitter: Emitter<ProcessErrorEvent> = new Emitter<ProcessErrorEvent>();
     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<IProcessStartEvent> {
         return this.startEmitter.event;
     }

diff --git 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
@@ -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
@@ -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<void> {
-        // 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<void> {
         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
@@ -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
@@ -29,6 +29,7 @@ export interface IBaseTerminalServer extends JsonRpcServer<IBaseTerminalClient>
     getProcessId(id: number): Promise<number>;
     getProcessInfo(id: number): Promise<TerminalProcessInfo>;
     getCwdURI(id: number): Promise<string>;
+    onAttached(id: number): Promise<void>;
     resize(id: number, cols: number, rows: number): Promise<void>;
     attach(id: number): Promise<number>;
     close(id: number): Promise<void>;
@@ -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
@@ -87,6 +87,17 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {
         };
     }
 
+    async onAttached(id: number): Promise<void> {
+        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<string> {
         const terminal = this.processManager.get(id);
         if (!(terminal instanceof TerminalProcess)) {