GH-815: Replace child_process with the Process API in the LS backend contribution

packages/core/src/node/backend-application.ts

@@ -19,6 +19,12 @@ export interface BackendApplicationContribution {
     initialize?(): void;
     configure?(app: express.Application): void;
     onStart?(server: http.Server): void;
+
+    /**
+     * Called when the backend application shuts down. Contributions must perform only synchronous operations.
+     * Any kind of additional asynchronous work queued in the event loop will be ignored and abandoned.
+     */
+    onStop?(app?: express.Application): void;
 }
 
 const defaultPort = BackendProcess.electron ? 0 : 3000;
@@ -64,6 +70,13 @@ export class BackendApplication {
             }
         });
 
+        // Handles normal process termination.
+        process.on('exit', () => this.onStop());
+        // Handles `Ctrl+C`.
+        process.on('SIGINT', () => this.onStop());
+        // Handles `kill pid`.
+        process.on('SIGTERM', () => this.onStop());
+
         for (const contribution of this.contributionsProvider.getContributions()) {
             if (contribution.initialize) {
                 try {
@@ -114,4 +127,16 @@ export class BackendApplication {
         return deferred.promise;
     }
 
+    private onStop(): void {
+        for (const contrib of this.contributionsProvider.getContributions()) {
+            if (contrib.onStop) {
+                try {
+                    contrib.onStop(this.app);
+                } catch (err) {
+                    this.logger.error(err);
+                }
+            }
+        }
+    }
+
 }

packages/java/src/browser/java-resource.ts

@@ -43,7 +43,7 @@ export class JavaResourceResolver implements ResourceResolver {
 
     resolve(uri: URI): JavaResource {
         if (uri.scheme !== JAVA_SCHEME) {
-            throw new Error("The given uri is not a java uri: " + uri);
+            throw new Error("The given URI is not a valid Java uri: " + uri);
         }
         return new JavaResource(uri, this.clientContribution);
     }

packages/java/src/node/java-contribution.ts

@@ -29,7 +29,7 @@ export class JavaContribution extends BaseLanguageServerContribution {
         const serverPath = path.resolve(__dirname, 'server');
         const jarPaths = glob.sync('**/plugins/org.eclipse.equinox.launcher_*.jar', { cwd: serverPath });
         if (jarPaths.length === 0) {
-            throw new Error('The java server launcher is not found.');
+            throw new Error('The Java server launcher is not found.');
         }
 
         const jarPath = path.resolve(serverPath, jarPaths[0]);
@@ -70,9 +70,8 @@ export class JavaContribution extends BaseLanguageServerContribution {
             env.STDIN_PORT = inServer.address().port;
             env.STDOUT_HOST = outServer.address().address;
             env.STDOUT_PORT = outServer.address().port;
-            this.createProcessSocketConnection(inSocket, outSocket, command, args, {
-                env: env
-            }).then(serverConnection => this.forward(clientConnection, serverConnection));
+            this.createProcessSocketConnection(inSocket, outSocket, command, args, { env })
+                .then(serverConnection => this.forward(clientConnection, serverConnection));
         });
     }
 }

packages/languages/package.json

@@ -4,6 +4,7 @@
   "description": "Theia - Languages Extension",
   "dependencies": {
     "@theia/core": "^0.2.2",
+    "@theia/process": "^0.2.2",
     "vscode-base-languageclient": "^0.0.1-alpha.3",
     "vscode-languageserver": "^3.4.0"
   },
@@ -46,4 +47,4 @@
   "nyc": {
     "extends": "../../configs/nyc.json"
   }
-}
+}

packages/languages/src/node/language-server-contribution.ts

@@ -7,17 +7,19 @@
 
 import * as net from 'net';
 import * as cp from 'child_process';
-import { injectable } from "inversify";
+import { injectable, inject } from "inversify";
 import { Message, isRequestMessage } from 'vscode-ws-jsonrpc';
 import { InitializeParams, InitializeRequest } from 'vscode-languageserver-protocol';
 import {
     createProcessSocketConnection,
-    createProcessStreamConnection,
+    createStreamConnection,
     forward,
     IConnection
 } from 'vscode-ws-jsonrpc/lib/server';
 import { MaybePromise } from "@theia/core/lib/common";
 import { LanguageContribution } from "../common";
+import { RawProcess, RawProcessFactory } from '@theia/process/lib/node/raw-process';
+import { ProcessManager } from '@theia/process/lib/node/process-manager';
 
 export {
     LanguageContribution, IConnection, Message
@@ -30,8 +32,16 @@ export interface LanguageServerContribution extends LanguageContribution {
 
 @injectable()
 export abstract class BaseLanguageServerContribution implements LanguageServerContribution {
+
     abstract readonly id: string;
     abstract readonly name: string;
+
+    @inject(RawProcessFactory)
+    protected readonly processFactory: RawProcessFactory;
+
+    @inject(ProcessManager)
+    protected readonly processManager: ProcessManager;
+
     abstract start(clientConnection: IConnection): void;
 
     protected forward(clientConnection: IConnection, serverConnection: IConnection): void {
@@ -48,27 +58,24 @@ export abstract class BaseLanguageServerContribution implements LanguageServerCo
         return message;
     }
 
-    protected createProcessSocketConnection(
-        outSocket: MaybePromise<net.Socket>, inSocket: MaybePromise<net.Socket>,
-        command: string, args?: string[], options?: cp.SpawnOptions
-    ): Promise<IConnection> {
+    protected async createProcessSocketConnection(outSocket: MaybePromise<net.Socket>, inSocket: MaybePromise<net.Socket>,
+        command: string, args?: string[], options?: cp.SpawnOptions): Promise<IConnection> {
+
         const process = this.spawnProcess(command, args, options);
-        return Promise.all([
-            Promise.resolve(outSocket),
-            Promise.resolve(inSocket)
-        ]).then(result => createProcessSocketConnection(process, result[0], result[1]));
+        const [outSock, inSock] = await Promise.all([outSocket, inSocket]);
+        return createProcessSocketConnection(process.process, outSock, inSock);
     }
 
     protected createProcessStreamConnection(command: string, args?: string[], options?: cp.SpawnOptions): IConnection {
         const process = this.spawnProcess(command, args, options);
-        return createProcessStreamConnection(process);
+        return createStreamConnection(process.output, process.input, () => process.kill());
     }
 
-    protected spawnProcess(command: string, args?: string[], options?: cp.SpawnOptions): cp.ChildProcess {
-        const serverProcess = cp.spawn(command, args, options);
-        serverProcess.once('error', this.onDidFailSpawnProcess.bind(this));
-        serverProcess.stderr.on('data', this.logError.bind(this));
-        return serverProcess;
+    protected spawnProcess(command: string, args?: string[], options?: cp.SpawnOptions): RawProcess {
+        const rawProcess = this.processFactory({ command, args, options });
+        rawProcess.process.once('error', this.onDidFailSpawnProcess.bind(this));
+        rawProcess.process.stderr.on('data', this.logError.bind(this));
+        return rawProcess;
     }
 
     protected onDidFailSpawnProcess(error: Error): void {

packages/languages/src/node/languages-backend-contribution.ts

@@ -12,13 +12,14 @@ import { ContributionProvider } from '@theia/core/lib/common';
 import { BackendApplicationContribution } from '@theia/core/lib/node';
 import { openJsonRpcSocket } from '@theia/core/lib/node';
 import { LanguageServerContribution, LanguageContribution } from "./language-server-contribution";
+import { ILogger } from '@theia/core/lib/common/logger';
 
 @injectable()
 export class LanguagesBackendContribution implements BackendApplicationContribution {
 
     constructor(
-        @inject(ContributionProvider) @named(LanguageServerContribution)
-        protected readonly contributors: ContributionProvider<LanguageServerContribution>
+        @inject(ContributionProvider) @named(LanguageServerContribution) protected readonly contributors: ContributionProvider<LanguageServerContribution>,
+        @inject(ILogger) protected logger: ILogger
     ) { }
 
     onStart(server: http.Server): void {
@@ -29,6 +30,7 @@ export class LanguagesBackendContribution implements BackendApplicationContribut
                     const connection = createWebSocketConnection(socket);
                     contribution.start(connection);
                 } catch (e) {
+                    this.logger.error(`Error occurred while starting language contribution. ${path}.`, e);
                     socket.dispose();
                     throw e;
                 }

packages/process/src/node/process-backend-module.ts

@@ -8,12 +8,14 @@
 import { ContainerModule, Container } from 'inversify';
 import { RawProcess, RawProcessOptions, RawProcessFactory } from './raw-process';
 import { TerminalProcess, TerminalProcessOptions, TerminalProcessFactory } from './terminal-process';
+import { BackendApplicationContribution } from '@theia/core/lib/node';
 import { ProcessManager } from "./process-manager";
 import { ILogger } from '@theia/core/lib/common';
 
 export default new ContainerModule(bind => {
     bind(RawProcess).toSelf().inTransientScope();
     bind(ProcessManager).toSelf().inSingletonScope();
+    bind(BackendApplicationContribution).to(ProcessManager).inSingletonScope();
     bind(RawProcessFactory).toFactory(ctx =>
         (options: RawProcessOptions) => {
             const child = new Container({ defaultScope: 'Singleton' });

packages/process/src/node/process-manager.ts

@@ -4,35 +4,80 @@
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  */
-import { injectable } from 'inversify';
+import { injectable, inject } from 'inversify';
 import { Process } from './process';
 import { Emitter, Event } from '@theia/core/lib/common';
+import { ILogger } from '@theia/core/lib/common/logger';
+import { BackendApplicationContribution } from '@theia/core/lib/node';
 
 @injectable()
-export class ProcessManager {
+export class ProcessManager implements BackendApplicationContribution {
 
-    protected readonly processes: Map<number, Process> = new Map();
     protected id: number = 0;
-    protected readonly deleteEmitter = new Emitter<number>();
+    protected readonly processes: Map<number, Process>;
+    protected readonly deleteEmitter: Emitter<number>;
 
+    constructor( @inject(ILogger) protected logger: ILogger) {
+        this.processes = new Map();
+        this.deleteEmitter = new Emitter<number>();
+    }
+
+    /**
+     * Registers the given process into this manager. Both on process termination and on error,
+     * the process will be automatically removed from the manager.
+     *
+     * @param process the process to register.
+     */
     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;
     }
 
-    get(id: number): Process | undefined {
-        return this.processes.get(id);
+    /**
+     * Removes the process from this process manager. Invoking this method, will make
+     * sure that the process is terminated before eliminating it from the manager's cache.
+     *
+     * @param process the process to unregister from this process manager.
+     */
+    protected unregister(process: Process): void {
+        const processLabel = this.getProcessLabel(process);
+        this.logger.debug(`Unregistering process. ${processLabel}`);
+        if (!process.killed) {
+            this.logger.debug(`Ensuring process termination. ${processLabel}`);
+            process.kill();
+        }
+        if (this.processes.delete(process.id)) {
+            this.deleteEmitter.fire(process.id);
+            this.logger.debug(`The process was successfully unregistered. ${processLabel}`);
+        } else {
+            this.logger.warn(`This process was not registered or was already unregistered. ${processLabel}`);
+        }
     }
 
-    delete(process: Process): void {
-        process.kill();
-        this.processes.delete(process.id);
-        this.deleteEmitter.fire(process.id);
+    get(id: number): Process | undefined {
+        return this.processes.get(id);
     }
 
     get onDelete(): Event<number> {
         return this.deleteEmitter.event;
     }
+
+    onStop?(): void {
+        for (const process of this.processes.values()) {
+            try {
+                this.unregister(process);
+            } catch (error) {
+                this.logger.error(`Error occurred when unregistering process. ${this.getProcessLabel(process)}`, error);
+            }
+        }
+    }
+
+    private getProcessLabel(process: Process): string {
+        return `[ID: ${process.id}]`;
+    }
+
 }

packages/process/src/node/process.ts

@@ -5,33 +5,38 @@
  * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  */
 
+import * as stream from 'stream';
 import { injectable, inject } from "inversify";
 import { ProcessManager } from './process-manager';
 import { ILogger, Emitter, Event } from '@theia/core/lib/common';
-import * as child from 'child_process';
-import * as stream from 'stream';
 
 export interface IProcessExitEvent {
-    code: number,
-    signal?: string
+    readonly code: number,
+    readonly signal?: string
+}
+
+export enum ProcessType {
+    'Raw',
+    'Terminal'
 }
 
 @injectable()
 export abstract class Process {
 
     readonly id: number;
-    abstract readonly type: 'Raw' | 'Terminal';
-    abstract pid: number;
-    abstract output: stream.Readable;
-    protected abstract process: child.ChildProcess | undefined;
-    protected abstract terminal: any;
-    protected readonly exitEmitter = new Emitter<IProcessExitEvent>();
-    protected readonly errorEmitter = new Emitter<Error>();
+    readonly exitEmitter: Emitter<IProcessExitEvent>;
+    readonly errorEmitter: Emitter<Error>;
+    abstract readonly pid: number;
+    abstract readonly output: stream.Readable;
     protected _killed = false;
 
     constructor(
         @inject(ProcessManager) protected readonly processManager: ProcessManager,
-        protected readonly logger: ILogger) {
+        protected readonly logger: ILogger,
+        readonly type: ProcessType) {
+
+        this.exitEmitter = new Emitter<IProcessExitEvent>();
+        this.errorEmitter = new Emitter<Error>();
         this.id = this.processManager.register(this);
     }
 
@@ -41,34 +46,24 @@ export abstract class Process {
         return this._killed;
     }
 
-    set killed(killed: boolean) {
-        /* readonly public property */
-    }
-
     get onExit(): Event<IProcessExitEvent> {
         return this.exitEmitter.event;
-
     }
 
     get onError(): Event<Error> {
         return this.errorEmitter.event;
-
     }
 
     protected emitOnExit(code: number, signal?: string) {
-        const exitEvent = { 'code': code, 'signal': signal };
+        const exitEvent = { code, signal };
         this.handleOnExit(exitEvent);
         this.exitEmitter.fire(exitEvent);
     }
 
     protected handleOnExit(event: IProcessExitEvent) {
         this._killed = true;
-        let logMsg = `Process ${this.pid} has exited with code ${event.code}`;
-
-        if (event.signal !== undefined) {
-            logMsg += `, signal : ${event.signal}.`;
-        }
-        this.logger.info(logMsg);
+        const signalSuffix = event.signal ? `, signal: ${event.signal}` : '';
+        this.logger.info(`Process ${this.pid} has exited with code ${event.code}${signalSuffix}.`);
     }
 
     protected emitOnError(err: Error) {
@@ -80,4 +75,5 @@ export abstract class Process {
         this._killed = true;
         this.logger.error(error.toString());
     }
+
 }