From 444566827a059eb9abc121885d5cceccb8a9f02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Fri, 20 Mar 2020 11:22:26 -0400 Subject: [PATCH] electron: fork backend in node process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify Electron's src-gen to always spawn the Theia backend in a Node.js process, and make sure to patch the `process.versions.electron` variable, for packages to correctly understand their environment (Electron vs Node). Add a debug configuration for both electron-main process and backend process. Signed-off-by: Paul Maréchal --- .vscode/launch.json | 69 +++++++++++++++---- .../src/generator/backend-generator.ts | 6 ++ .../src/generator/frontend-generator.ts | 65 +++++++++-------- .../application-package/src/environment.ts | 11 +-- 4 files changed, 95 insertions(+), 56 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 5a5a5af3b346e..f807d219ff1dd 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -19,30 +19,65 @@ { "type": "node", "request": "launch", - "name": "Launch Electron Backend", - "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/electron", - "windows": { - "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/electron.cmd" - }, + "name": "Launch Electron Main Process", + "runtimeExecutable": "yarn", + "runtimeArgs": [ + "electron", + "--inspect-brk=9228", // This config connects to port 9228. + "--remote-debugging-port=9222", // Allow attaching to browser-windows. + ], "cwd": "${workspaceRoot}/examples/electron", "protocol": "inspector", + "port": 9228, "args": [ ".", "--log-level=debug", "--hostname=localhost", "--no-cluster", "--app-project-path=${workspaceRoot}/examples/electron", - "--remote-debugging-port=9222", "--no-app-auto-install" ], "env": { "NODE_ENV": "development", + "THEIA_BACKEND_EXEC_ARGV": "[\"--inspect=9229\"]", // Pass debug flags to backend process. "THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}" }, "sourceMaps": true, "outFiles": [ "${workspaceRoot}/examples/electron/src-gen/frontend/electron-main.js", - "${workspaceRoot}/examples/electron/src-gen/backend/main.js", + "${workspaceRoot}/examples/electron/src-gen/backend/main.js" + ], + "smartStep": true, + "internalConsoleOptions": "openOnSessionStart", + "outputCapture": "std" + }, + { + "type": "node", + "request": "launch", + "name": "Launch Electron Node Backend", + "runtimeExecutable": "yarn", + "runtimeArgs": [ + "electron", + "--remote-debugging-port=9222", // Allow attaching to browser-windows. + ], + "cwd": "${workspaceRoot}/examples/electron", + "protocol": "inspector", + "port": 9229, + "args": [ + ".", + "--log-level=debug", + "--hostname=localhost", + "--no-cluster", + "--app-project-path=${workspaceRoot}/examples/electron", + "--no-app-auto-install" + ], + "env": { + "NODE_ENV": "development", + "THEIA_BACKEND_EXEC_ARGV": "[\"--inspect-brk=9229\"]", // Pass debug flags to backend process. + "THEIA_WEBVIEW_EXTERNAL_ENDPOINT": "${env:THEIA_WEBVIEW_EXTERNAL_ENDPOINT}" + }, + "sourceMaps": true, + "outFiles": [ "${workspaceRoot}/examples/electron/lib/**/*.js", "${workspaceRoot}/packages/*/lib/**/*.js", "${workspaceRoot}/dev-packages/*/lib/**/*.js" @@ -51,6 +86,18 @@ "internalConsoleOptions": "openOnSessionStart", "outputCapture": "std" }, + { + "type": "node", + "request": "attach", + "name": "Attach to Electron Node Backend", + "port": 9229 + }, + { + "type": "chrome", + "request": "attach", + "name": "Attach to Electron Frontend", + "port": 9222 + }, { "type": "node", "request": "launch", @@ -149,12 +196,6 @@ "url": "http://localhost:3000/", "webRoot": "${workspaceRoot}" }, - { - "type": "chrome", - "request": "attach", - "name": "Attach to Electron Frontend", - "port": 9222 - }, { "name": "Launch VS Code Tests", "type": "node", @@ -184,7 +225,7 @@ { "name": "Launch Electron Backend & Frontend", "configurations": [ - "Launch Electron Backend", + "Launch Electron Node Backend", "Attach to Electron Frontend" ] } 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..fd30418c5a0f1 100644 --- a/dev-packages/application-manager/src/generator/frontend-generator.ts +++ b/dev-packages/application-manager/src/generator/frontend-generator.ts @@ -325,43 +325,42 @@ app.on('ready', () => { // https://github.com/eclipse-theia/theia/issues/3297#issuecomment-439172274 process.env.THEIA_APP_PROJECT_PATH = resolve(__dirname, '..', '..'); - // 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; + // Used for debugging: + // The backend runs in a separate process, we may want to pass options to it. + const theiaNodeBackendExecArgv = [...process.execArgv]; + if (typeof process.env.THEIA_BACKEND_EXEC_ARGV === 'string') { + const parsedExecArgv = JSON.parse(process.env.THEIA_BACKEND_EXEC_ARGV); + if (!Array.isArray(parsedExecArgv)) { + throw new TypeError('process.env.THEIA_BACKEND_EXEC_ARGV should be a serialized JSON array.'); + } + theiaNodeBackendExecArgv.push(...parsedExecArgv); + delete process.env.THEIA_BACKEND_EXEC_ARGV; } 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) { - process.env[ElectronSecurityToken] = JSON.stringify(electronSecurityToken); - require(mainPath).then(address => { - loadMainWindow(address.port); - }).catch((error) => { - console.error(error); - app.exit(1); - }); - } else { - const cp = fork(mainPath, [], { env: Object.assign({ + const theiaNodeBackend = fork(mainPath, [], { + execArgv: theiaNodeBackendExecArgv, + env: Object.assign({ [ElectronSecurityToken]: JSON.stringify(electronSecurityToken), - }, process.env) }); - cp.on('message', (address) => { - loadMainWindow(address.port); - }); - cp.on('error', (error) => { - console.error(error); - app.exit(1); - }); - app.on('quit', () => { - // If we forked the process for the clusters, we need to manually terminate it. - // See: https://github.com/eclipse-theia/theia/issues/835 - process.kill(cp.pid); - }); - } + /** + * 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. + * The forked backend should patch its \`process.versions.electron\` with this value if it is missing. + */ + THEIA_ELECTRON_VERSION: process.versions.electron, + }, process.env) + }); + theiaNodeBackend.on('message', (address) => { + loadMainWindow(address.port); + }); + theiaNodeBackend.on('error', (error) => { + console.error(error); + app.exit(1); + }); + app.on('quit', () => { + // Avoid process leak, see: https://github.com/eclipse-theia/theia/issues/835 + process.kill(theiaNodeBackend.pid); + }); }); `; } diff --git a/dev-packages/application-package/src/environment.ts b/dev-packages/application-package/src/environment.ts index a6439ce4a5eec..07dc933748804 100644 --- a/dev-packages/application-package/src/environment.ts +++ b/dev-packages/application-package/src/environment.ts @@ -21,20 +21,13 @@ const isElectron: () => boolean = require('is-electron'); */ class ElectronEnv { - /** - * Environment variable that can be accessed on the `process` to check if running in electron or not. - */ - readonly THEIA_ELECTRON_VERSION = 'THEIA_ELECTRON_VERSION'; - /** * `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(); } /**