GH-835: Made sure forked process is terminated on application quit.

Adjusted server worker log messages, logged PID too. Signed-off-by: Akos Kitta <kittaakos@gmail.com>
eclipse-theia · Nov 16, 2017 · 2ceb174 · 2ceb174
1 parent 2a75607
commit 2ceb174
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 13 deletions.
diff --git a/dev-packages/application-package/src/generator/frontend-generator.ts b/dev-packages/application-package/src/generator/frontend-generator.ts
@@ -93,13 +93,13 @@ if (cluster.isMaster) {
     });
     electron.app.on('ready', function () {
         const path = require('path');
-        const { fork }  = require('child_process');
+        const { fork } = require('child_process');
         // Check whether we are in bundled application or development mode.
         const devMode = process.defaultApp || /node_modules[\\/]electron[\\/]/.test(process.execPath);
         const mainWindow = new electron.BrowserWindow({ width: 1024, height: 728, show: false });
         mainWindow.on('ready-to-show', () => mainWindow.show());
         const mainPath = path.join(__dirname, '..', 'backend', 'main');
-        const loadMainWindow = function(port) {
+        const loadMainWindow = function (port) {
             mainWindow.loadURL(\`file://\${path.join(__dirname, '../../lib/index.html')}?port=\${port}\`);
         };
         // We need to distinguish between bundled application and development mode when starting the clusters.
@@ -120,6 +120,11 @@ if (cluster.isMaster) {
                 console.error(error);
                 electron.app.exit(1);
             });
+            electron.app.on('quit', function() {
+                // If we forked the process for the clusters, we need to manually terminate it.
+                // See: https://github.com/theia-ide/theia/issues/835
+                process.kill(cp.pid);
+            });
         }
         mainWindow.on('closed', function () {
             electron.app.exit(0);

diff --git a/packages/core/src/node/cluster/master-process.ts b/packages/core/src/node/cluster/master-process.ts
@@ -19,7 +19,7 @@ export class MasterProcess extends EventEmitter {
 
     start(): ServerWorker {
         if (this.serverWorker) {
-            throw new Error('The express server worker is already running.')
+            throw new Error('Server worker is already running.');
         }
         this.serverWorker = this.fork();
         this.emit('started', this.serverWorker);
@@ -31,10 +31,10 @@ export class MasterProcess extends EventEmitter {
 
     async restart(): Promise<void> {
         if (!this.serverWorker) {
-            throw new Error('The express server worker is not running.');
+            throw new Error('Server worker is not running.');
         }
         this.emit('restarting', this.serverWorker);
-        console.log(`Restarting the express server worker is requested.`);
+        console.log(`Restarting the server worker is requested.`);
         const serverWorker = this.fork();
         const success = serverWorker.initialized.then(() => true);
 
@@ -44,12 +44,12 @@ export class MasterProcess extends EventEmitter {
         const restarted = await Promise.race([success, failure]);
         if (!restarted) {
             serverWorker.stop();
-            const message = `The express server worker failed to restart.`;
+            const message = `Server worker failed to restart.`;
             console.error(message);
             throw new Error(message);
         }
         this.serverWorker = serverWorker;
-        console.log(`The express server worker has been restarted.`);
+        console.log(`Server worker has been restarted.`);
         this.emit('restarted', this.serverWorker);
     }
     get restarting(): Promise<ServerWorker> {

diff --git a/packages/core/src/node/cluster/server-worker.ts b/packages/core/src/node/cluster/server-worker.ts
@@ -25,7 +25,7 @@ export class ServerWorker {
         let onDidInitialize: () => void = () => { };
         this.initialized = new Promise<void>(resolve => onDidInitialize = resolve);
 
-        console.log('Starting the express server worker...');
+        console.log('Starting server worker...');
         this.worker = cluster.fork();
         this.server = createRemoteServer(this.worker, { onDidInitialize, restart });
 
@@ -35,11 +35,12 @@ export class ServerWorker {
         this.disconnect = new Promise(resolve => this.worker.once('disconnect', resolve));
         this.exit = new Promise(resolve => this.worker.once('exit', resolve));
 
-        this.online.then(() => console.log(`The express server worker ${this.worker.id} has been started.`));
-        this.failed.then(error => console.error(`The express server worker ${this.worker.id} failed:`, error));
-        this.initialized.then(() => console.log(`The express server worker ${this.worker.id} is ready to accept messages.`));
-        this.disconnect.then(() => console.log(`The express server worker ${this.worker.id} has been disconnected.`));
-        this.exit.then(() => console.log(`The express server worker ${this.worker.id} has been stopped.`));
+        const workerIdentifier = `[ID: ${this.worker.id} | PID: ${this.worker.process.pid}]`;
+        this.online.then(() => console.log(`Server worker has been started. ${workerIdentifier}`));
+        this.failed.then(error => console.error(`Server worker failed. ${workerIdentifier}`, error));
+        this.initialized.then(() => console.log(`Server worker is ready to accept messages. ${workerIdentifier}`));
+        this.disconnect.then(() => console.log(`Server worker has been disconnected. ${workerIdentifier}`));
+        this.exit.then(() => console.log(`Server worker has been stopped. ${workerIdentifier}`));
     }
 
     async stop(): Promise<void> {