From d55e06936bab67d1c364fec61dfc5b01c3ff33cc Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 18 Nov 2020 11:43:25 -0600 Subject: [PATCH 1/4] Split child and parent wrappers I think having them combined and relying on if statements was getting confusing especially if we want to add additional messages with different payloads (which will soon be the case). --- src/node/entry.ts | 13 ++- src/node/vscode.ts | 4 +- src/node/wrapper.ts | 232 ++++++++++++++++++++++++++------------------ 3 files changed, 146 insertions(+), 103 deletions(-) diff --git a/src/node/entry.ts b/src/node/entry.ts index b661b184..8fcacc0d 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -19,7 +19,7 @@ import { coderCloudBind } from "./coder-cloud" import { commit, version } from "./constants" import { register } from "./routes" import { humanPath, isFile, open } from "./util" -import { ipcMain, WrapperProcess } from "./wrapper" +import { isChild, wrapper } from "./wrapper" export const runVsCodeCli = (args: DefaultedArgs): void => { logger.debug("forking vs code cli...") @@ -137,7 +137,7 @@ const main = async (args: DefaultedArgs): Promise => { logger.info(" - Connected to cloud agent") } catch (err) { logger.error(err.message) - ipcMain.exit(1) + wrapper.exit(1) } } @@ -161,9 +161,9 @@ async function entry(): Promise { // There's no need to check flags like --help or to spawn in an existing // instance for the child process because these would have already happened in // the parent and the child wouldn't have been spawned. - if (ipcMain.isChild) { - await ipcMain.handshake() - ipcMain.preventExit() + if (isChild(wrapper)) { + await wrapper.handshake() + wrapper.preventExit() return main(args) } @@ -201,11 +201,10 @@ async function entry(): Promise { return openInExistingInstance(args, socketPath) } - const wrapper = new WrapperProcess(require("../../package.json").version) return wrapper.start() } entry().catch((error) => { logger.error(error.message) - ipcMain.exit(error) + wrapper.exit(error) }) diff --git a/src/node/vscode.ts b/src/node/vscode.ts index c3da8a24..23282443 100644 --- a/src/node/vscode.ts +++ b/src/node/vscode.ts @@ -8,7 +8,7 @@ import { rootPath } from "./constants" import { settings } from "./settings" import { SocketProxyProvider } from "./socket" import { isFile } from "./util" -import { ipcMain } from "./wrapper" +import { wrapper } from "./wrapper" export class VscodeProvider { public readonly serverRootPath: string @@ -20,7 +20,7 @@ export class VscodeProvider { public constructor() { this.vsRootPath = path.resolve(rootPath, "lib/vscode") this.serverRootPath = path.join(this.vsRootPath, "out/vs/server") - ipcMain.onDispose(() => this.dispose()) + wrapper.onDispose(() => this.dispose()) } public async dispose(): Promise { diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index 2e8c51cd..88035efb 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -1,4 +1,4 @@ -import { field, logger } from "@coder/logger" +import { Logger, field, logger } from "@coder/logger" import * as cp from "child_process" import * as path from "path" import * as rfs from "rotating-file-stream" @@ -14,9 +14,9 @@ interface RelaunchMessage { version: string } -export type Message = RelaunchMessage | HandshakeMessage +type Message = RelaunchMessage | HandshakeMessage -export class ProcessError extends Error { +class ProcessError extends Error { public constructor(message: string, public readonly code: number | undefined) { super(message) this.name = this.constructor.name @@ -25,16 +25,26 @@ export class ProcessError extends Error { } /** - * Allows the wrapper and inner processes to communicate. + * Wrapper around a process that tries to gracefully exit when a process exits + * and provides a way to prevent `process.exit`. */ -export class IpcMain { - private readonly _onMessage = new Emitter() - public readonly onMessage = this._onMessage.event - private readonly _onDispose = new Emitter() - public readonly onDispose = this._onDispose.event - public readonly processExit: (code?: number) => never = process.exit +abstract class Process { + /** + * Emit this to trigger a graceful exit. + */ + protected readonly _onDispose = new Emitter() - public constructor(private readonly parentPid?: number) { + /** + * Emitted when the process is about to be disposed. + */ + public readonly onDispose = this._onDispose.event + + /** + * Uniquely named logger for the process. + */ + public abstract logger: Logger + + public constructor() { process.on("SIGINT", () => this._onDispose.emit("SIGINT")) process.on("SIGTERM", () => this._onDispose.emit("SIGTERM")) process.on("exit", () => this._onDispose.emit(undefined)) @@ -43,42 +53,27 @@ export class IpcMain { // Remove listeners to avoid possibly triggering disposal again. process.removeAllListeners() - // Try waiting for other handlers run first then exit. - logger.debug(`${parentPid ? "inner process" : "wrapper"} ${process.pid} disposing`, field("code", signal)) + // Try waiting for other handlers to run first then exit. + this.logger.debug("disposing", field("code", signal)) wait.then(() => this.exit(0)) setTimeout(() => this.exit(0), 5000) }) - - // Kill the inner process if the parent dies. This is for the case where the - // parent process is forcefully terminated and cannot clean up. - if (parentPid) { - setInterval(() => { - try { - // process.kill throws an exception if the process doesn't exist. - process.kill(parentPid, 0) - } catch (_) { - // Consider this an error since it should have been able to clean up - // the child process unless it was forcefully killed. - logger.error(`parent process ${parentPid} died`) - this._onDispose.emit(undefined) - } - }, 5000) - } } /** - * Ensure we control when the process exits. + * Ensure control over when the process exits. */ public preventExit(): void { - process.exit = function (code?: number) { - logger.warn(`process.exit() was prevented: ${code || "unknown code"}.`) - } as (code?: number) => never + ;(process.exit as any) = (code?: number) => { + this.logger.warn(`process.exit() was prevented: ${code || "unknown code"}.`) + } } - public get isChild(): boolean { - return typeof this.parentPid !== "undefined" - } + private readonly processExit: (code?: number) => never = process.exit + /** + * Will always exit even if normal exit is being prevented. + */ public exit(error?: number | ProcessError): never { if (error && typeof error !== "number") { this.processExit(typeof error.code === "number" ? error.code : 1) @@ -86,47 +81,61 @@ export class IpcMain { this.processExit(error) } } +} - public handshake(child?: cp.ChildProcess): Promise { - return new Promise((resolve, reject) => { - const target = child || process +/** + * Child process that will clean up after itself if the parent goes away and can + * perform a handshake with the parent and ask it to relaunch. + */ +class ChildProcess extends Process { + public logger = logger.named(`child:${process.pid}`) + + public constructor(private readonly parentPid: number) { + super() + + // Kill the inner process if the parent dies. This is for the case where the + // parent process is forcefully terminated and cannot clean up. + setInterval(() => { + try { + // process.kill throws an exception if the process doesn't exist. + process.kill(this.parentPid, 0) + } catch (_) { + // Consider this an error since it should have been able to clean up + // the child process unless it was forcefully killed. + this.logger.error(`parent process ${parentPid} died`) + this._onDispose.emit(undefined) + } + }, 5000) + } + + /** + * Initiate the handshake and wait for a response from the parent. + */ + public handshake(): Promise { + return new Promise((resolve) => { const onMessage = (message: Message): void => { - logger.debug( - `${child ? "wrapper" : "inner process"} ${process.pid} received message from ${ - child ? child.pid : this.parentPid - }`, - field("message", message), - ) + logger.debug(`received message from ${this.parentPid}`, field("message", message)) if (message.type === "handshake") { - target.removeListener("message", onMessage) - target.on("message", (msg) => this._onMessage.emit(msg)) - // The wrapper responds once the inner process starts the handshake. - if (child) { - if (!target.send) { - throw new Error("child not spawned with IPC") - } - target.send({ type: "handshake" }) - } + process.removeListener("message", onMessage) resolve() } } - target.on("message", onMessage) - if (child) { - child.once("error", reject) - child.once("exit", (code) => { - reject(new ProcessError(`Unexpected exit with code ${code}`, code !== null ? code : undefined)) - }) - } else { - // The inner process initiates the handshake. - this.send({ type: "handshake" }) - } + // Initiate the handshake and wait for the reply. + process.on("message", onMessage) + this.send({ type: "handshake" }) }) } + /** + * Notify the parent process that it should relaunch the child. + */ public relaunch(version: string): void { this.send({ type: "relaunch", version }) } + /** + * Send a message to the parent. + */ private send(message: Message): void { if (!process.send) { throw new Error("not spawned with IPC") @@ -135,29 +144,30 @@ export class IpcMain { } } -/** - * Channel for communication between the child and parent processes. - */ -export const ipcMain = new IpcMain( - typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" ? parseInt(process.env.CODE_SERVER_PARENT_PID) : undefined, -) - export interface WrapperOptions { maxMemory?: number nodeOptions?: string } /** - * Provides a way to wrap a process for the purpose of updating the running - * instance. + * Parent process wrapper that spawns the child process and performs a handshake + * with it. Will relaunch the child if it receives a SIGUSR1 or is asked to by + * the child. If the child otherwise exits the parent will also exit. */ -export class WrapperProcess { - private process?: cp.ChildProcess +export class ParentProcess extends Process { + public logger = logger.named(`parent:${process.pid}`) + + private child?: cp.ChildProcess private started?: Promise private readonly logStdoutStream: rfs.RotatingFileStream private readonly logStderrStream: rfs.RotatingFileStream + protected readonly _onChildMessage = new Emitter() + protected readonly onChildMessage = this._onChildMessage.event + public constructor(private currentVersion: string, private readonly options?: WrapperOptions) { + super() + const opts = { size: "10M", maxFiles: 10, @@ -165,19 +175,19 @@ export class WrapperProcess { this.logStdoutStream = rfs.createStream(path.join(paths.data, "coder-logs", "code-server-stdout.log"), opts) this.logStderrStream = rfs.createStream(path.join(paths.data, "coder-logs", "code-server-stderr.log"), opts) - ipcMain.onDispose(() => { + this.onDispose(() => { this.disposeChild() }) - ipcMain.onMessage((message) => { + this.onChildMessage((message) => { switch (message.type) { case "relaunch": - logger.info(`Relaunching: ${this.currentVersion} -> ${message.version}`) + this.logger.info(`Relaunching: ${this.currentVersion} -> ${message.version}`) this.currentVersion = message.version this.relaunch() break default: - logger.error(`Unrecognized message ${message}`) + this.logger.error(`Unrecognized message ${message}`) break } }) @@ -185,9 +195,9 @@ export class WrapperProcess { private disposeChild(): void { this.started = undefined - if (this.process) { - this.process.removeAllListeners() - this.process.kill() + if (this.child) { + this.child.removeAllListeners() + this.child.kill() } } @@ -196,16 +206,16 @@ export class WrapperProcess { try { await this.start() } catch (error) { - logger.error(error.message) - ipcMain.exit(typeof error.code === "number" ? error.code : 1) + this.logger.error(error.message) + this.exit(typeof error.code === "number" ? error.code : 1) } } public start(): Promise { // If we have a process then we've already bound this. - if (!this.process) { + if (!this.child) { process.on("SIGUSR1", async () => { - logger.info("Received SIGUSR1; hotswapping") + this.logger.info("Received SIGUSR1; hotswapping") this.relaunch() }) } @@ -217,7 +227,7 @@ export class WrapperProcess { private async _start(): Promise { const child = this.spawn() - this.process = child + this.child = child // Log both to stdout and to the log directory. if (child.stdout) { @@ -229,13 +239,13 @@ export class WrapperProcess { child.stderr.pipe(process.stderr) } - logger.debug(`spawned inner process ${child.pid}`) + this.logger.debug(`spawned inner process ${child.pid}`) - await ipcMain.handshake(child) + await this.handshake(child) child.once("exit", (code) => { - logger.debug(`inner process ${child.pid} exited unexpectedly`) - ipcMain.exit(code || 0) + this.logger.debug(`inner process ${child.pid} exited unexpectedly`) + this.exit(code || 0) }) } @@ -256,18 +266,52 @@ export class WrapperProcess { stdio: ["ipc"], }) } + + /** + * Wait for a handshake from the child then reply. + */ + private handshake(child: cp.ChildProcess): Promise { + return new Promise((resolve, reject) => { + const onMessage = (message: Message): void => { + logger.debug(`received message from ${child.pid}`, field("message", message)) + if (message.type === "handshake") { + child.removeListener("message", onMessage) + child.on("message", (msg) => this._onChildMessage.emit(msg)) + child.send({ type: "handshake" }) + resolve() + } + } + child.on("message", onMessage) + child.once("error", reject) + child.once("exit", (code) => { + reject(new ProcessError(`Unexpected exit with code ${code}`, code !== null ? code : undefined)) + }) + }) + } +} + +/** + * Process wrapper. + */ +export const wrapper = + typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" + ? new ChildProcess(parseInt(process.env.CODE_SERVER_PARENT_PID)) + : new ParentProcess(require("../../package.json").version) + +export function isChild(proc: ChildProcess | ParentProcess): proc is ChildProcess { + return proc instanceof ChildProcess } // It's possible that the pipe has closed (for example if you run code-server // --version | head -1). Assume that means we're done. if (!process.stdout.isTTY) { - process.stdout.on("error", () => ipcMain.exit()) + process.stdout.on("error", () => wrapper.exit()) } // Don't let uncaught exceptions crash the process. process.on("uncaughtException", (error) => { - logger.error(`Uncaught exception: ${error.message}`) + wrapper.logger.error(`Uncaught exception: ${error.message}`) if (typeof error.stack !== "undefined") { - logger.error(error.stack) + wrapper.logger.error(error.stack) } }) From 247c4ec77623048e0a08a2e785f78e5972d591de Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 18 Nov 2020 12:05:24 -0600 Subject: [PATCH 2/4] Move onMessage so it can be used in the wrappers --- src/node/vscode.ts | 66 +++++------------------------ src/node/wrapper.ts | 101 +++++++++++++++++++++++++++++++------------- 2 files changed, 81 insertions(+), 86 deletions(-) diff --git a/src/node/vscode.ts b/src/node/vscode.ts index 23282443..3c18cdee 100644 --- a/src/node/vscode.ts +++ b/src/node/vscode.ts @@ -1,4 +1,4 @@ -import { field, logger } from "@coder/logger" +import { logger } from "@coder/logger" import * as cp from "child_process" import * as net from "net" import * as path from "path" @@ -8,13 +8,12 @@ import { rootPath } from "./constants" import { settings } from "./settings" import { SocketProxyProvider } from "./socket" import { isFile } from "./util" -import { wrapper } from "./wrapper" +import { onMessage, wrapper } from "./wrapper" export class VscodeProvider { public readonly serverRootPath: string public readonly vsRootPath: string private _vscode?: Promise - private timeoutInterval = 10000 // 10s, matches VS Code's timeouts. private readonly socketProvider = new SocketProxyProvider() public constructor() { @@ -69,10 +68,13 @@ export class VscodeProvider { vscode, ) - const message = await this.onMessage(vscode, (message): message is ipc.OptionsMessage => { - // There can be parallel initializations so wait for the right ID. - return message.type === "options" && message.id === id - }) + const message = await onMessage( + vscode, + (message): message is ipc.OptionsMessage => { + // There can be parallel initializations so wait for the right ID. + return message.type === "options" && message.id === id + }, + ) return message.options } @@ -104,61 +106,13 @@ export class VscodeProvider { dispose() }) - this._vscode = this.onMessage(vscode, (message): message is ipc.ReadyMessage => { + this._vscode = onMessage(vscode, (message): message is ipc.ReadyMessage => { return message.type === "ready" }).then(() => vscode) return this._vscode } - /** - * Listen to a single message from a process. Reject if the process errors, - * exits, or times out. - * - * `fn` is a function that determines whether the message is the one we're - * waiting for. - */ - private onMessage( - proc: cp.ChildProcess, - fn: (message: ipc.VscodeMessage) => message is T, - ): Promise { - return new Promise((resolve, reject) => { - const cleanup = () => { - proc.off("error", onError) - proc.off("exit", onExit) - proc.off("message", onMessage) - clearTimeout(timeout) - } - - const timeout = setTimeout(() => { - cleanup() - reject(new Error("timed out")) - }, this.timeoutInterval) - - const onError = (error: Error) => { - cleanup() - reject(error) - } - - const onExit = (code: number | null) => { - cleanup() - reject(new Error(`VS Code exited unexpectedly with code ${code}`)) - } - - const onMessage = (message: ipc.VscodeMessage) => { - logger.trace("got message from vscode", field("message", message)) - if (fn(message)) { - cleanup() - resolve(message) - } - } - - proc.on("message", onMessage) - proc.on("error", onError) - proc.on("exit", onExit) - }) - } - /** * VS Code expects a raw socket. It will handle all the web socket frames. */ diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index 88035efb..5933725e 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -5,6 +5,59 @@ import * as rfs from "rotating-file-stream" import { Emitter } from "../common/emitter" import { paths } from "./util" +const timeoutInterval = 10000 // 10s, matches VS Code's timeouts. + +/** + * Listen to a single message from a process. Reject if the process errors, + * exits, or times out. + * + * `fn` is a function that determines whether the message is the one we're + * waiting for. + */ +export function onMessage( + proc: cp.ChildProcess | NodeJS.Process, + fn: (message: M) => message is T, + customLogger?: Logger, +): Promise { + return new Promise((resolve, reject) => { + const cleanup = () => { + proc.off("error", onError) + proc.off("exit", onExit) + proc.off("message", onMessage) + clearTimeout(timeout) + } + + const timeout = setTimeout(() => { + cleanup() + reject(new Error("timed out")) + }, timeoutInterval) + + const onError = (error: Error) => { + cleanup() + reject(error) + } + + const onExit = (code: number) => { + cleanup() + reject(new Error(`exited unexpectedly with code ${code}`)) + } + + const onMessage = (message: M) => { + ;(customLogger || logger).trace("got message", field("message", message)) + if (fn(message)) { + cleanup() + resolve(message) + } + } + + proc.on("message", onMessage) + // NodeJS.Process doesn't have `error` but binding anyway shouldn't break + // anything. It does have `exit` but the types aren't working. + ;(proc as cp.ChildProcess).on("error", onError) + ;(proc as cp.ChildProcess).on("exit", onExit) + }) +} + interface HandshakeMessage { type: "handshake" } @@ -111,19 +164,15 @@ class ChildProcess extends Process { /** * Initiate the handshake and wait for a response from the parent. */ - public handshake(): Promise { - return new Promise((resolve) => { - const onMessage = (message: Message): void => { - logger.debug(`received message from ${this.parentPid}`, field("message", message)) - if (message.type === "handshake") { - process.removeListener("message", onMessage) - resolve() - } - } - // Initiate the handshake and wait for the reply. - process.on("message", onMessage) - this.send({ type: "handshake" }) - }) + public async handshake(): Promise { + this.send({ type: "handshake" }) + await onMessage( + process, + (message): message is HandshakeMessage => { + return message.type === "handshake" + }, + this.logger, + ) } /** @@ -270,23 +319,15 @@ export class ParentProcess extends Process { /** * Wait for a handshake from the child then reply. */ - private handshake(child: cp.ChildProcess): Promise { - return new Promise((resolve, reject) => { - const onMessage = (message: Message): void => { - logger.debug(`received message from ${child.pid}`, field("message", message)) - if (message.type === "handshake") { - child.removeListener("message", onMessage) - child.on("message", (msg) => this._onChildMessage.emit(msg)) - child.send({ type: "handshake" }) - resolve() - } - } - child.on("message", onMessage) - child.once("error", reject) - child.once("exit", (code) => { - reject(new ProcessError(`Unexpected exit with code ${code}`, code !== null ? code : undefined)) - }) - }) + private async handshake(child: cp.ChildProcess): Promise { + await onMessage( + child, + (message): message is HandshakeMessage => { + return message.type === "handshake" + }, + this.logger, + ) + child.send({ type: "handshake" }) } } From 016daf2fdd5056b29b6f80ab9fbeac55b218e2de Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 18 Nov 2020 13:01:46 -0600 Subject: [PATCH 3/4] Parse arguments once Fixes #2316. --- src/node/entry.ts | 17 +++++++------ src/node/wrapper.ts | 61 ++++++++++++++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/src/node/entry.ts b/src/node/entry.ts index 8fcacc0d..9aa69a65 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -154,19 +154,22 @@ const main = async (args: DefaultedArgs): Promise => { } async function entry(): Promise { - const cliArgs = parse(process.argv.slice(2)) - const configArgs = await readConfigFile(cliArgs.config) - const args = await setDefaults(cliArgs, configArgs) - // There's no need to check flags like --help or to spawn in an existing // instance for the child process because these would have already happened in - // the parent and the child wouldn't have been spawned. + // the parent and the child wouldn't have been spawned. We also get the + // arguments from the parent so we don't have to parse twice and to account + // for environment manipulation (like how PASSWORD gets removed to avoid + // leaking to child processes). if (isChild(wrapper)) { - await wrapper.handshake() + const args = await wrapper.handshake() wrapper.preventExit() return main(args) } + const cliArgs = parse(process.argv.slice(2)) + const configArgs = await readConfigFile(cliArgs.config) + const args = await setDefaults(cliArgs, configArgs) + if (args.help) { console.log("code-server", version, commit) console.log("") @@ -201,7 +204,7 @@ async function entry(): Promise { return openInExistingInstance(args, socketPath) } - return wrapper.start() + return wrapper.start(args) } entry().catch((error) => { diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index 5933725e..b63d6057 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -1,8 +1,9 @@ -import { Logger, field, logger } from "@coder/logger" +import { field, Logger, logger } from "@coder/logger" import * as cp from "child_process" import * as path from "path" import * as rfs from "rotating-file-stream" import { Emitter } from "../common/emitter" +import { DefaultedArgs } from "./cli" import { paths } from "./util" const timeoutInterval = 10000 // 10s, matches VS Code's timeouts. @@ -58,7 +59,12 @@ export function onMessage( }) } -interface HandshakeMessage { +interface ParentHandshakeMessage { + type: "handshake" + args: DefaultedArgs +} + +interface ChildHandshakeMessage { type: "handshake" } @@ -67,7 +73,8 @@ interface RelaunchMessage { version: string } -type Message = RelaunchMessage | HandshakeMessage +type ChildMessage = RelaunchMessage | ChildHandshakeMessage +type ParentMessage = ParentHandshakeMessage class ProcessError extends Error { public constructor(message: string, public readonly code: number | undefined) { @@ -164,15 +171,16 @@ class ChildProcess extends Process { /** * Initiate the handshake and wait for a response from the parent. */ - public async handshake(): Promise { + public async handshake(): Promise { this.send({ type: "handshake" }) - await onMessage( + const message = await onMessage( process, - (message): message is HandshakeMessage => { + (message): message is ParentHandshakeMessage => { return message.type === "handshake" }, this.logger, ) + return message.args } /** @@ -185,7 +193,7 @@ class ChildProcess extends Process { /** * Send a message to the parent. */ - private send(message: Message): void { + private send(message: ChildMessage): void { if (!process.send) { throw new Error("not spawned with IPC") } @@ -211,12 +219,19 @@ export class ParentProcess extends Process { private readonly logStdoutStream: rfs.RotatingFileStream private readonly logStderrStream: rfs.RotatingFileStream - protected readonly _onChildMessage = new Emitter() + protected readonly _onChildMessage = new Emitter() protected readonly onChildMessage = this._onChildMessage.event + private args?: DefaultedArgs + public constructor(private currentVersion: string, private readonly options?: WrapperOptions) { super() + process.on("SIGUSR1", async () => { + this.logger.info("Received SIGUSR1; hotswapping") + this.relaunch() + }) + const opts = { size: "10M", maxFiles: 10, @@ -253,21 +268,17 @@ export class ParentProcess extends Process { private async relaunch(): Promise { this.disposeChild() try { - await this.start() + this.started = this._start() + await this.started } catch (error) { this.logger.error(error.message) this.exit(typeof error.code === "number" ? error.code : 1) } } - public start(): Promise { - // If we have a process then we've already bound this. - if (!this.child) { - process.on("SIGUSR1", async () => { - this.logger.info("Received SIGUSR1; hotswapping") - this.relaunch() - }) - } + public start(args: DefaultedArgs): Promise { + // Store for relaunches. + this.args = args if (!this.started) { this.started = this._start() } @@ -320,14 +331,24 @@ export class ParentProcess extends Process { * Wait for a handshake from the child then reply. */ private async handshake(child: cp.ChildProcess): Promise { - await onMessage( + if (!this.args) { + throw new Error("started without args") + } + await onMessage( child, - (message): message is HandshakeMessage => { + (message): message is ChildHandshakeMessage => { return message.type === "handshake" }, this.logger, ) - child.send({ type: "handshake" }) + this.send(child, { type: "handshake", args: this.args }) + } + + /** + * Send a message to the child. + */ + private send(child: cp.ChildProcess, message: ParentMessage): void { + child.send(message) } } From 95ef6dbf2f9eaf999406878d195e3318721c1f3c Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 18 Nov 2020 13:08:57 -0600 Subject: [PATCH 4/4] Remove unused wrapper options Also move our memory default to the beginning of NODE_OPTIONS so it can be overidden. The version of the flag with dashes seems to be the more correct one now so use that instead of underscores. Related: #2113. --- src/node/wrapper.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index b63d6057..f6f84e2b 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -201,11 +201,6 @@ class ChildProcess extends Process { } } -export interface WrapperOptions { - maxMemory?: number - nodeOptions?: string -} - /** * Parent process wrapper that spawns the child process and performs a handshake * with it. Will relaunch the child if it receives a SIGUSR1 or is asked to by @@ -224,7 +219,7 @@ export class ParentProcess extends Process { private args?: DefaultedArgs - public constructor(private currentVersion: string, private readonly options?: WrapperOptions) { + public constructor(private currentVersion: string) { super() process.on("SIGUSR1", async () => { @@ -310,18 +305,12 @@ export class ParentProcess extends Process { } private spawn(): cp.ChildProcess { - // Flags to pass along to the Node binary. - let nodeOptions = `${process.env.NODE_OPTIONS || ""} ${(this.options && this.options.nodeOptions) || ""}` - if (!/max_old_space_size=(\d+)/g.exec(nodeOptions)) { - nodeOptions += ` --max_old_space_size=${(this.options && this.options.maxMemory) || 2048}` - } - // Use spawn (instead of fork) to use the new binary in case it was updated. return cp.spawn(process.argv[0], process.argv.slice(1), { env: { ...process.env, CODE_SERVER_PARENT_PID: process.pid.toString(), - NODE_OPTIONS: nodeOptions, + NODE_OPTIONS: `--max-old-space-size=2048 ${process.env.NODE_OPTIONS || ""}`, }, stdio: ["ipc"], })