From 97e5e66009247574c17ce13ca776309a04376b99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Fri, 20 Mar 2020 15:00:39 -0400 Subject: [PATCH] electron: fix backend forking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes 3 issues: 1. When forking, Electron 4 forgets to set `process.versions.electron` in the sub-process, making some packages wrongly guess their environment. This commit manually patches this variable. 2. The generated application code also used to not fork the backend when working from sources. Running the backend in the Electron main process was only meant to help with debugging. This commit makes the generated code fork the backend unless a `--no-cluster` flag is specified. 3. When running a bundled application, the backend was forked. But any command line parameter passed to the Electron application wasn't passing any argument down to the backend. This commit makes sure that relevant arguments are passed to the forked backend process. Signed-off-by: Paul Maréchal --- dev-packages/application-manager/README.md | 17 ++++++++++++++ .../src/generator/backend-generator.ts | 6 +++++ .../src/generator/frontend-generator.ts | 22 ++++++++++--------- .../application-package/src/environment.ts | 6 ++--- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/dev-packages/application-manager/README.md b/dev-packages/application-manager/README.md index 5e73ea18b8404..58f38947124f9 100644 --- a/dev-packages/application-manager/README.md +++ b/dev-packages/application-manager/README.md @@ -10,6 +10,23 @@ +## README + +This package contains the generators used when creating a Theia application through `@theia/cli`. + +This package can auto-generate application code for both the backend and frontend, as well as webpack configuration files. + +When targeting Electron, the `electron-main.js` script will spawn the backend process in a Node.js sub-process, where Electron's API won't be available. +To prevent the generated application from forking the backend, you can pass a `--no-cluster` flag, or set an environment variable like `THEIA_ELECTRON_NO_BACKEND_FORK=1`. + +```sh +# when developing a Theia application with @theia/cli: +yarn theia start --no-cluster + +# when starting a bundled application made using @theia/cli: +bundled-application.exe --no-cluster +``` + ## Additional Information - [Theia - GitHub](https://github.com/eclipse-theia/theia) diff --git a/dev-packages/application-manager/src/generator/backend-generator.ts b/dev-packages/application-manager/src/generator/backend-generator.ts index d82b797f839d8..62304ebaa5637 100644 --- a/dev-packages/application-manager/src/generator/backend-generator.ts +++ b/dev-packages/application-manager/src/generator/backend-generator.ts @@ -27,6 +27,12 @@ export class BackendGenerator extends AbstractGenerator { protected compileServer(backendModules: Map): string { return `// @ts-check require('reflect-metadata'); + +// Patch electron version if missing, see https://github.com/eclipse-theia/theia/pull/7361#pullrequestreview-377065146 +if (typeof process.versions.electron === 'undefined' && typeof process.env.THEIA_ELECTRON_VERSION === 'string') { + process.versions.electron = process.env.THEIA_ELECTRON_VERSION; +} + const path = require('path'); const express = require('express'); const { Container } = require('inversify'); diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts index 6556fa4d3fc88..f2d5d2d8962e7 100644 --- a/dev-packages/application-manager/src/generator/frontend-generator.ts +++ b/dev-packages/application-manager/src/generator/frontend-generator.ts @@ -290,6 +290,8 @@ app.on('ready', () => { // Check whether we are in bundled application or development mode. // @ts-ignore const devMode = process.defaultApp || /node_modules[\/]electron[\/]/.test(process.execPath); + // Check if user wants to run everything as one process. + const noBackendFork = Boolean(process.env['THEIA_ELECTRON_NO_BACKEND_FORK']) || process.argv.includes('--no-cluster'); const mainWindow = createNewWindow(); if (isSingleInstance) { @@ -327,17 +329,14 @@ app.on('ready', () => { // Set the electron version for both the dev and the production mode. (https://github.com/eclipse-theia/theia/issues/3254) // Otherwise, the forked backend processes will not know that they're serving the electron frontend. - const { versions } = process; - // @ts-ignore - if (versions && typeof versions.electron !== 'undefined') { - // @ts-ignore - process.env.THEIA_ELECTRON_VERSION = versions.electron; - } + // The forked backend should patch its \`process.versions.electron\` with this value if it is missing. + process.env.THEIA_ELECTRON_VERSION = process.versions.electron; const mainPath = join(__dirname, '..', 'backend', 'main'); - // We need to distinguish between bundled application and development mode when starting the clusters. - // See: https://github.com/electron/electron/issues/6337#issuecomment-230183287 - if (devMode) { + // We spawn a separate process for the backend for Express to not run in the Electron main process. + // See: https://github.com/eclipse-theia/theia/pull/7361#issuecomment-601272212 + // But when in debugging we want to run everything in the same process to make things easier. + if (noBackendFork) { process.env[ElectronSecurityToken] = JSON.stringify(electronSecurityToken); require(mainPath).then(address => { loadMainWindow(address.port); @@ -346,7 +345,10 @@ app.on('ready', () => { app.exit(1); }); } else { - const cp = fork(mainPath, [], { env: Object.assign({ + // We want to pass flags passed to the Electron app to the backend process. + // Quirk: When developing from sources, we execute Electron as \`electron.exe electron-main.js ...args\`, but when bundled, + // the command looks like \`bundled-application.exe ...args\`. + const cp = fork(mainPath, process.argv.slice(devMode ? 2 : 1), { env: Object.assign({ [ElectronSecurityToken]: JSON.stringify(electronSecurityToken), }, process.env) }); cp.on('message', (address) => { diff --git a/dev-packages/application-package/src/environment.ts b/dev-packages/application-package/src/environment.ts index a6439ce4a5eec..8e185eaca68d6 100644 --- a/dev-packages/application-package/src/environment.ts +++ b/dev-packages/application-package/src/environment.ts @@ -29,12 +29,10 @@ class ElectronEnv { /** * `true` if running in electron. Otherwise, `false`. * - * Can be called from both the `main` and the render process. Also works for forked cluster workers. + * Can be called from both the `main` and the render process. */ is(): boolean { - // When forking a new process from the cluster, we can rely neither on `process.versions` nor `process.argv`. - // Se we look into the `process.env` as well. `is-electron` does not do it for us. - return isElectron() || typeof process !== 'undefined' && typeof process.env === 'object' && !!process.env.THEIA_ELECTRON_VERSION; + return isElectron(); } /**