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

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

Hide file tree

Showing 9 changed files with 57 additions and 12 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;
+    public exited = false;
+    public attached = false;
 
     /**
      * The OS process id.

diff --git a/packages/process/src/node/raw-process.ts b/packages/process/src/node/raw-process.ts
@@ -128,6 +128,7 @@ export class RawProcess extends Process {
                     typeof exitCode === 'number' ? exitCode : undefined,
                     typeof signal === 'string' ? signal : undefined,
                 );
+                this.processManager.unregister(this);
             });
 
             this.process.on('close', (exitCode, signal) => {

diff --git a/packages/process/src/node/terminal-process.ts b/packages/process/src/node/terminal-process.ts
@@ -106,6 +106,13 @@ export class TerminalProcess extends Process {
                 } else {
                     this.emitOnExit(undefined, signame(signal));
                 }
+                this.onProcessExited();
+
+                // Unregister process only if terminal already attached, Fix issue #2961.
+                if (this.attached) {
+                    this.unregisterProcess();
+                }
+
                 process.nextTick(() => {
                     if (signal === undefined || signal === 0) {
                         this.emitOnClose(code, undefined);
@@ -168,6 +175,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
@@ -65,6 +65,7 @@ import { PROBLEMS_WIDGET_ID, ProblemWidget } from '@theia/markers/lib/browser/pr
 import { TaskNode } from './task-node';
 import { MonacoWorkspace } from '@theia/monaco/lib/browser/monaco-workspace';
 import { TaskTerminalWidgetManager } from './task-terminal-widget-manager';
+import { ShellTerminalServerProxy } from '@theia/terminal/lib/common/shell-terminal-protocol';
 
 export interface QuickPickProblemMatcherItem {
     problemMatchers: NamedProblemMatcher[] | undefined;
@@ -157,6 +158,9 @@ export class TaskService implements TaskConfigurationClient {
     @inject(OpenerService)
     protected readonly openerService: OpenerService;
 
+    @inject(ShellTerminalServerProxy)
+    protected readonly shellTerminalServer: ShellTerminalServerProxy;
+
     @inject(TaskNameResolver)
     protected readonly taskNameResolver: TaskNameResolver;
 
@@ -986,8 +990,9 @@ export class TaskService implements TaskConfigurationClient {
     protected async runResolvedTask(resolvedTask: TaskConfiguration, option?: RunTaskOption): Promise<TaskInfo | undefined> {
         const source = resolvedTask._source;
         const taskLabel = resolvedTask.label;
+        let taskInfo: TaskInfo | undefined;
         try {
-            const taskInfo = await this.taskServer.run(resolvedTask, this.getContext(), option);
+            taskInfo = await this.taskServer.run(resolvedTask, this.getContext(), option);
             this.lastTask = { source, taskLabel, scope: resolvedTask._scope };
             this.logger.debug(`Task created. Task id: ${taskInfo.taskId}`);
 
@@ -998,13 +1003,16 @@ 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);
+                await this.attach(taskInfo.terminalId, taskInfo);
             }
             return taskInfo;
         } catch (error) {
             const errorStr = `Error launching task '${taskLabel}': ${error.message}`;
             this.logger.error(errorStr);
             this.messageService.error(errorStr);
+            if (taskInfo && typeof taskInfo.terminalId === 'number') {
+                this.shellTerminalServer.onFailedAttach(taskInfo.terminalId);
+            }
         }
     }
 
@@ -1059,11 +1067,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<number | void> {
         let widgetOpenMode: WidgetOpenMode = 'open';
         if (taskInfo) {
             const terminalWidget = this.terminalService.getByTerminalId(terminalId);
@@ -1079,6 +1083,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(),
@@ -1092,7 +1097,7 @@ export class TaskService implements TaskConfigurationClient {
             mode: widgetOpenMode,
             taskInfo
         });
-        widget.start(terminalId);
+        return widget.start(terminalId);
     }
 
     protected getTerminalWidgetId(terminalId: number): string | undefined {

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
@@ -31,6 +31,8 @@ export interface IBaseTerminalServer extends JsonRpcServer<IBaseTerminalClient>
     getCwdURI(id: number): Promise<string>;
     resize(id: number, cols: number, rows: number): Promise<void>;
     attach(id: number): Promise<number>;
+    onAttached(id: number): Promise<void>;
+    onFailedAttach(id: number): Promise<void>;
     close(id: number): Promise<void>;
     getDefaultShell(): Promise<string>;
 
@@ -171,7 +173,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
@@ -68,6 +68,26 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {
         }
     }
 
+    async onAttached(id: number): Promise<void> {
+        const terminal = this.processManager.get(id);
+        if (terminal instanceof TerminalProcess) {
+            terminal.attached = true;
+            if (terminal.exited) {
+                // Although terminal exited - waited with the `unregisterProcess` until here to enable attaching (Fix issue #2961).
+                terminal.unregisterProcess();
+            }
+        }
+    }
+
+    async onFailedAttach(id: number): Promise<void> {
+        const terminal = this.processManager.get(id);
+        if (terminal instanceof TerminalProcess) {
+            if (terminal.exited) {
+                terminal.unregisterProcess();
+            };
+        }
+    }
+
     async getProcessId(id: number): Promise<number> {
         const terminal = this.processManager.get(id);
         if (!(terminal instanceof TerminalProcess)) {