Skip to content

Separate process wrappers and pass arguments #2334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions src/node/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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...")
Expand Down Expand Up @@ -137,7 +137,7 @@ const main = async (args: DefaultedArgs): Promise<void> => {
logger.info(" - Connected to cloud agent")
} catch (err) {
logger.error(err.message)
ipcMain.exit(1)
wrapper.exit(1)
}
}

Expand All @@ -154,19 +154,22 @@ const main = async (args: DefaultedArgs): Promise<void> => {
}

async function entry(): Promise<void> {
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.
if (ipcMain.isChild) {
await ipcMain.handshake()
ipcMain.preventExit()
// 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)) {
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("")
Expand Down Expand Up @@ -201,11 +204,10 @@ async function entry(): Promise<void> {
return openInExistingInstance(args, socketPath)
}

const wrapper = new WrapperProcess(require("../../package.json").version)
return wrapper.start()
return wrapper.start(args)
}

entry().catch((error) => {
logger.error(error.message)
ipcMain.exit(error)
wrapper.exit(error)
})
68 changes: 11 additions & 57 deletions src/node/vscode.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -8,19 +8,18 @@ import { rootPath } from "./constants"
import { settings } from "./settings"
import { SocketProxyProvider } from "./socket"
import { isFile } from "./util"
import { ipcMain } from "./wrapper"
import { onMessage, wrapper } from "./wrapper"

export class VscodeProvider {
public readonly serverRootPath: string
public readonly vsRootPath: string
private _vscode?: Promise<cp.ChildProcess>
private timeoutInterval = 10000 // 10s, matches VS Code's timeouts.
private readonly socketProvider = new SocketProxyProvider()

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<void> {
Expand Down Expand Up @@ -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<ipc.VscodeMessage, ipc.OptionsMessage>(
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
}
Expand Down Expand Up @@ -104,61 +106,13 @@ export class VscodeProvider {
dispose()
})

this._vscode = this.onMessage(vscode, (message): message is ipc.ReadyMessage => {
this._vscode = onMessage<ipc.VscodeMessage, ipc.ReadyMessage>(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<T extends ipc.VscodeMessage>(
proc: cp.ChildProcess,
fn: (message: ipc.VscodeMessage) => message is T,
): Promise<T> {
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.
*/
Expand Down
Loading