From fe19391c036bf234e84b2e30f321de73f3b689e2 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 15 Sep 2020 16:51:43 -0500 Subject: [PATCH] Read most recent socket path from file --- ci/dev/vscode.patch | 25 +++++++++++++ src/node/cli.ts | 32 +++++++++++++++- src/node/socket.ts | 13 +------ src/node/util.ts | 15 ++++++++ test/cli.test.ts | 89 +++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 156 insertions(+), 18 deletions(-) diff --git a/ci/dev/vscode.patch b/ci/dev/vscode.patch index b3a7289df..f15c7d7a7 100644 --- a/ci/dev/vscode.patch +++ b/ci/dev/vscode.patch @@ -3035,6 +3035,31 @@ index b3c89e51cfc25a53293a352a2a8ad50d5f26d595..e21abe4e13bc25a5b72f556bbfb61085 registerSingleton(IExtHostTerminalService, ExtHostTerminalService); registerSingleton(IExtHostTunnelService, ExtHostTunnelService); +registerSingleton(IExtHostNodeProxy, class extends NotImplementedProxy(String(IExtHostNodeProxy)) { whenReady = Promise.resolve(); }); +diff --git a/src/vs/workbench/api/node/extHostCLIServer.ts b/src/vs/workbench/api/node/extHostCLIServer.ts +index 7cae126cc0f804273850933468690e0f9f10a5b8..08c2aa5cdae3f3d06bb08b7055dc7e7def260132 100644 +--- a/src/vs/workbench/api/node/extHostCLIServer.ts ++++ b/src/vs/workbench/api/node/extHostCLIServer.ts +@@ -11,6 +11,8 @@ import { IWindowOpenable, IOpenWindowOptions } from 'vs/platform/windows/common/ + import { URI } from 'vs/base/common/uri'; + import { hasWorkspaceFileExtension } from 'vs/platform/workspaces/common/workspaces'; + import { ILogService } from 'vs/platform/log/common/log'; ++import { join } from 'vs/base/common/path'; ++import { tmpdir } from 'os'; + + export interface OpenCommandPipeArgs { + type: 'open'; +@@ -54,6 +56,11 @@ export class CLIServer { + private async setup(): Promise { + this._ipcHandlePath = generateRandomPipeName(); + ++ // NOTE@coder: Write this out so we can get the most recent path. ++ fs.promises.writeFile(join(tmpdir(), "vscode-ipc"), this._ipcHandlePath).catch((error) => { ++ this.logService.error(error); ++ }); ++ + try { + this._server.listen(this.ipcHandlePath); + this._server.on('error', err => this.logService.error(err)); diff --git a/src/vs/workbench/api/worker/extHost.worker.services.ts b/src/vs/workbench/api/worker/extHost.worker.services.ts index 3843fdec386edc09a1d361b63de892a04e0070ed..8aac4df527857e964798362a69f5591bef07c165 100644 --- a/src/vs/workbench/api/worker/extHost.worker.services.ts diff --git a/src/node/cli.ts b/src/node/cli.ts index a52796a14..028ea0d45 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -5,7 +5,7 @@ import * as os from "os" import * as path from "path" import { Args as VsArgs } from "../../lib/vscode/src/vs/server/ipc" import { AuthType } from "./http" -import { generatePassword, humanPath, paths } from "./util" +import { canConnect, generatePassword, humanPath, paths } from "./util" export class Optional { public constructor(public readonly value?: T) {} @@ -512,7 +512,35 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise => { + try { + return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") + } catch (error) { + if (error.code !== "ENOENT") { + throw error + } + } + return undefined + } + + // If these flags are set then assume the user is trying to open in an + // existing instance since these flags have no effect otherwise. + const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => { + return args[cur as keyof Args] ? prev + 1 : prev + }, 0) + if (openInFlagCount > 0) { + return readSocketPath() + } + + // It's possible the user is trying to spawn another instance of code-server. + // Check if any unrelated flags are set (add one for `_` which always exists), + // that a file or directory was passed, and that the socket is active. + if (Object.keys(args).length === openInFlagCount + 1 && args._.length > 0) { + const socketPath = await readSocketPath() + if (socketPath && (await canConnect(socketPath))) { + return socketPath + } + } return undefined } diff --git a/src/node/socket.ts b/src/node/socket.ts index e5fe66778..ada024831 100644 --- a/src/node/socket.ts +++ b/src/node/socket.ts @@ -4,7 +4,7 @@ import * as path from "path" import * as tls from "tls" import { Emitter } from "../common/emitter" import { generateUuid } from "../common/util" -import { tmpdir } from "./util" +import { canConnect, tmpdir } from "./util" /** * Provides a way to proxy a TLS socket. Can be used when you need to pass a @@ -89,17 +89,6 @@ export class SocketProxyProvider { } public async findFreeSocketPath(basePath: string, maxTries = 100): Promise { - const canConnect = (path: string): Promise => { - return new Promise((resolve) => { - const socket = net.connect(path) - socket.once("error", () => resolve(false)) - socket.once("connect", () => { - socket.destroy() - resolve(true) - }) - }) - } - let i = 0 let path = basePath while ((await canConnect(path)) && i < maxTries) { diff --git a/src/node/util.ts b/src/node/util.ts index c0f37f74b..75122fe76 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -2,6 +2,7 @@ import * as cp from "child_process" import * as crypto from "crypto" import envPaths from "env-paths" import * as fs from "fs-extra" +import * as net from "net" import * as os from "os" import * as path from "path" import * as util from "util" @@ -246,3 +247,17 @@ export function pathToFsPath(path: string, keepDriveLetterCasing = false): strin } return value } + +/** + * Return a promise that resolves with whether the socket path is active. + */ +export function canConnect(path: string): Promise { + return new Promise((resolve) => { + const socket = net.connect(path) + socket.once("error", () => resolve(false)) + socket.once("connect", () => { + socket.destroy() + resolve(true) + }) + }) +} diff --git a/test/cli.test.ts b/test/cli.test.ts index fe78659d8..ae5256142 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -1,16 +1,24 @@ import { Level, logger } from "@coder/logger" import * as assert from "assert" +import * as fs from "fs-extra" +import * as net from "net" +import * as os from "os" import * as path from "path" -import { parse, setDefaults } from "../src/node/cli" -import { paths } from "../src/node/util" +import { Args, parse, setDefaults, shouldOpenInExistingInstance } from "../src/node/cli" +import { paths, tmpdir } from "../src/node/util" -describe("cli", () => { +type Mutable = { + -readonly [P in keyof T]: T[P] +} + +describe("parser", () => { beforeEach(() => { delete process.env.LOG_LEVEL }) // The parser should not set any defaults so the caller can determine what - // values the user actually set. These are set after calling `setDefaults`. + // values the user actually set. These are only set after explicitly calling + // `setDefaults`. const defaults = { "extensions-dir": path.join(paths.data, "extensions"), "user-data-dir": paths.data, @@ -225,3 +233,76 @@ describe("cli", () => { }) }) }) + +describe("cli", () => { + let args: Mutable = { _: [] } + const testDir = path.join(tmpdir, "tests/cli") + const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") + + before(async () => { + await fs.remove(testDir) + await fs.mkdirp(testDir) + }) + + beforeEach(async () => { + delete process.env.VSCODE_IPC_HOOK_CLI + args = { _: [] } + await fs.remove(vscodeIpcPath) + }) + + it("should use existing if inside code-server", async () => { + process.env.VSCODE_IPC_HOOK_CLI = "test" + assert.strictEqual(await shouldOpenInExistingInstance(args), "test") + + args.port = 8081 + args._.push("./file") + assert.strictEqual(await shouldOpenInExistingInstance(args), "test") + }) + + it("should use existing if --reuse-window is set", async () => { + args["reuse-window"] = true + assert.strictEqual(await shouldOpenInExistingInstance(args), undefined) + + await fs.writeFile(vscodeIpcPath, "test") + assert.strictEqual(await shouldOpenInExistingInstance(args), "test") + + args.port = 8081 + assert.strictEqual(await shouldOpenInExistingInstance(args), "test") + }) + + it("should use existing if --new-window is set", async () => { + args["new-window"] = true + assert.strictEqual(await shouldOpenInExistingInstance(args), undefined) + + await fs.writeFile(vscodeIpcPath, "test") + assert.strictEqual(await shouldOpenInExistingInstance(args), "test") + + args.port = 8081 + assert.strictEqual(await shouldOpenInExistingInstance(args), "test") + }) + + it("should use existing if no unrelated flags are set, has positional, and socket is active", async () => { + assert.strictEqual(await shouldOpenInExistingInstance(args), undefined) + + args._.push("./file") + assert.strictEqual(await shouldOpenInExistingInstance(args), undefined) + + const socketPath = path.join(testDir, "socket") + await fs.writeFile(vscodeIpcPath, socketPath) + assert.strictEqual(await shouldOpenInExistingInstance(args), undefined) + + await new Promise((resolve) => { + const server = net.createServer(() => { + // Close after getting the first connection. + server.close() + }) + server.once("listening", () => resolve(server)) + server.listen(socketPath) + }) + + assert.strictEqual(await shouldOpenInExistingInstance(args), socketPath) + + args.port = 8081 + assert.strictEqual(await shouldOpenInExistingInstance(args), undefined) + }) +})