From f50cfb89848d9995589c3fccabd413d26ee6994d Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 30 May 2026 23:51:35 +0900 Subject: [PATCH] fix(shared): prune Bun directory fds before spawn --- src/cli/doctor/spawn-with-timeout.test.ts | 10 +- src/shared/bun-spawn-shim.ts | 11 +- .../command-executor/execute-hook-command.ts | 2 +- src/shared/spawn-fd-pruner.test.ts | 139 ++++++++++++++++++ src/shared/spawn-fd-pruner.ts | 94 ++++++++++++ src/shared/tmux/runner.test.ts | 17 ++- src/shared/tmux/runner.ts | 6 +- 7 files changed, 263 insertions(+), 16 deletions(-) create mode 100644 src/shared/spawn-fd-pruner.test.ts create mode 100644 src/shared/spawn-fd-pruner.ts diff --git a/src/cli/doctor/spawn-with-timeout.test.ts b/src/cli/doctor/spawn-with-timeout.test.ts index 099b42402..14eca68c3 100644 --- a/src/cli/doctor/spawn-with-timeout.test.ts +++ b/src/cli/doctor/spawn-with-timeout.test.ts @@ -1,11 +1,13 @@ import { describe, it, expect } from "bun:test" import { spawnWithTimeout } from "./spawn-with-timeout" +const SHELL_BIN = process.platform === "win32" ? "sh" : "/bin/sh" + describe("spawnWithTimeout", () => { describe("#given a command that completes quickly", () => { it("returns stdout and exit code", async () => { // when - const result = await spawnWithTimeout(["echo", "hello"], { stdout: "pipe", stderr: "pipe" }) + const result = await spawnWithTimeout([SHELL_BIN, "-c", "printf '%s\\n' hello"], { stdout: "pipe", stderr: "pipe" }) // then expect(result.timedOut).toBe(false) @@ -19,7 +21,7 @@ describe("spawnWithTimeout", () => { it("captures stderr output", async () => { // when const result = await spawnWithTimeout( - ["bash", "-c", "echo err >&2"], + [SHELL_BIN, "-c", "printf '%s\\n' err >&2"], { stdout: "pipe", stderr: "pipe" } ) @@ -32,7 +34,7 @@ describe("spawnWithTimeout", () => { describe("#given a command that fails", () => { it("returns non-zero exit code without timing out", async () => { // when - const result = await spawnWithTimeout(["false"], { stdout: "pipe", stderr: "pipe" }) + const result = await spawnWithTimeout([SHELL_BIN, "-c", "exit 1"], { stdout: "pipe", stderr: "pipe" }) // then expect(result.timedOut).toBe(false) @@ -44,7 +46,7 @@ describe("spawnWithTimeout", () => { it("returns timedOut true and kills the process", async () => { // when const result = await spawnWithTimeout( - ["bash", "-c", "while true; do :; done"], + [SHELL_BIN, "-c", "while true; do :; done"], { stdout: "pipe", stderr: "pipe" }, 200 ) diff --git a/src/shared/bun-spawn-shim.ts b/src/shared/bun-spawn-shim.ts index 36260135e..19e32b977 100644 --- a/src/shared/bun-spawn-shim.ts +++ b/src/shared/bun-spawn-shim.ts @@ -5,6 +5,7 @@ import { type SpawnSyncOptions as NodeSpawnSyncOptions, } from "node:child_process" import { Readable, Writable } from "node:stream" +import { pruneLeakedDirectoryFileDescriptorsBeforeSpawn } from "./spawn-fd-pruner" type AnyRecord = Record type StdioMode = "pipe" | "inherit" | "ignore" @@ -190,7 +191,10 @@ export function spawn(options: SpawnOptions & { cmd: string[] }): SpawnedProcess export function spawn(cmdOrOpts: unknown, opts?: unknown): SpawnedProcess { const { cmd, opts: options } = resolveCommand(cmdOrOpts, opts) const bun = getBunRuntime() - if (bun) return bun.spawn(cmd, options) + if (bun) { + pruneLeakedDirectoryFileDescriptorsBeforeSpawn() + return bun.spawn(cmd, options) + } const [bin, ...args] = cmd if (bin === undefined) { @@ -207,7 +211,10 @@ export function spawnSync(options: SpawnOptions & { cmd: string[] }): SpawnSyncR export function spawnSync(cmdOrOpts: unknown, opts?: unknown): SpawnSyncResult { const { cmd, opts: options } = resolveCommand(cmdOrOpts, opts) const bun = getBunRuntime() - if (bun) return bun.spawnSync(cmd, options) + if (bun) { + pruneLeakedDirectoryFileDescriptorsBeforeSpawn() + return bun.spawnSync(cmd, options) + } const [bin, ...args] = cmd if (bin === undefined) { diff --git a/src/shared/command-executor/execute-hook-command.ts b/src/shared/command-executor/execute-hook-command.ts index 43c628f2f..0501b115b 100644 --- a/src/shared/command-executor/execute-hook-command.ts +++ b/src/shared/command-executor/execute-hook-command.ts @@ -79,7 +79,7 @@ export async function executeHookCommand( const proc = spawn(finalCommand, { cwd, - shell: true, + shell: isWin32 ? true : "/bin/sh", detached: !isWin32, env, }); diff --git a/src/shared/spawn-fd-pruner.test.ts b/src/shared/spawn-fd-pruner.test.ts new file mode 100644 index 000000000..ce663c09b --- /dev/null +++ b/src/shared/spawn-fd-pruner.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, test } from "bun:test" + +import { + pruneLeakedDirectoryFileDescriptors, + type SpawnFdPrunerDependencies, +} from "./spawn-fd-pruner" + +class FileDescriptorError extends Error { + readonly code: string + + constructor(code: string) { + super(code) + this.code = code + } +} + +function createStats(isDirectory: boolean): { isDirectory(): boolean } { + return { + isDirectory: () => isDirectory, + } +} + +describe("pruneLeakedDirectoryFileDescriptors", () => { + test("#given darwin Bun process above threshold #when pruning #then only directory descriptors are closed", () => { + // given + const closedDescriptors: number[] = [] + const dependencies = { + platform: "darwin", + isBunRuntime: true, + readdirSync: () => ["0", "1", "2", "3", "4", "not-a-fd", "5"], + fstatSync: (fd: number) => createStats(fd !== 4), + closeSync: (fd: number) => { + closedDescriptors.push(fd) + }, + } satisfies SpawnFdPrunerDependencies + + // when + const result = pruneLeakedDirectoryFileDescriptors(dependencies, { threshold: 5 }) + + // then + expect(closedDescriptors).toEqual([3, 5]) + expect(result).toEqual({ skipped: false, inspectedCount: 3, closedCount: 2 }) + }) + + test("#given fd count below threshold #when pruning #then it skips descriptor inspection", () => { + // given + const dependencies = { + platform: "darwin", + isBunRuntime: true, + readdirSync: () => ["0", "1", "2", "3"], + fstatSync: () => { + throw new Error("fstatSync should not run below threshold") + }, + closeSync: () => { + throw new Error("closeSync should not run below threshold") + }, + } satisfies SpawnFdPrunerDependencies + + // when + const result = pruneLeakedDirectoryFileDescriptors(dependencies, { threshold: 10 }) + + // then + expect(result).toEqual({ skipped: true, inspectedCount: 0, closedCount: 0 }) + }) + + test("#given non-darwin or non-Bun process #when pruning #then it no-ops", () => { + // given + const createDependencies = ( + platform: SpawnFdPrunerDependencies["platform"], + isBunRuntime: boolean, + ): SpawnFdPrunerDependencies => ({ + platform, + isBunRuntime, + readdirSync: () => { + throw new Error("readdirSync should not run outside darwin Bun") + }, + fstatSync: () => createStats(true), + closeSync: () => {}, + }) + + // when + const linuxResult = pruneLeakedDirectoryFileDescriptors(createDependencies("linux", true), { threshold: 1 }) + const nodeResult = pruneLeakedDirectoryFileDescriptors(createDependencies("darwin", false), { threshold: 1 }) + + // then + expect(linuxResult).toEqual({ skipped: true, inspectedCount: 0, closedCount: 0 }) + expect(nodeResult).toEqual({ skipped: true, inspectedCount: 0, closedCount: 0 }) + }) + + test("#given transient fd scan errors #when pruning #then it ignores them and keeps closing other directories", () => { + // given + const closedDescriptors: number[] = [] + const dependencies = { + platform: "darwin", + isBunRuntime: true, + readdirSync: () => ["3", "4", "5", "6"], + fstatSync: (fd: number) => { + if (fd === 3) { + throw new FileDescriptorError("EBADF") + } + + return createStats(true) + }, + closeSync: (fd: number) => { + if (fd === 5) { + throw new FileDescriptorError("EINVAL") + } + + closedDescriptors.push(fd) + }, + } satisfies SpawnFdPrunerDependencies + + // when + const result = pruneLeakedDirectoryFileDescriptors(dependencies, { threshold: 1 }) + + // then + expect(closedDescriptors).toEqual([4, 6]) + expect(result).toEqual({ skipped: false, inspectedCount: 4, closedCount: 2 }) + }) + + test("#given unexpected fd scan error #when pruning #then it rethrows", () => { + // given + const dependencies = { + platform: "darwin", + isBunRuntime: true, + readdirSync: () => ["3"], + fstatSync: () => { + throw new FileDescriptorError("EPERM") + }, + closeSync: () => {}, + } satisfies SpawnFdPrunerDependencies + + // when + const prune = () => pruneLeakedDirectoryFileDescriptors(dependencies, { threshold: 1 }) + + // then + expect(prune).toThrow("EPERM") + }) +}) diff --git a/src/shared/spawn-fd-pruner.ts b/src/shared/spawn-fd-pruner.ts new file mode 100644 index 000000000..80a4a5427 --- /dev/null +++ b/src/shared/spawn-fd-pruner.ts @@ -0,0 +1,94 @@ +import { closeSync, fstatSync, readdirSync } from "node:fs" + +const FD_DIRECTORY = "/dev/fd" +const DEFAULT_PRUNE_THRESHOLD = 8192 +const IGNORED_FD_ERROR_CODES = new Set(["EBADF", "EINVAL", "ENOENT"]) + +type FileDescriptorStats = { + isDirectory(): boolean +} + +export type SpawnFdPrunerDependencies = { + readonly platform: NodeJS.Platform + readonly isBunRuntime: boolean + readonly readdirSync: (path: string) => string[] + readonly fstatSync: (fd: number) => FileDescriptorStats + readonly closeSync: (fd: number) => void +} + +export type SpawnFdPruneOptions = { + readonly threshold?: number +} + +export type SpawnFdPruneResult = { + readonly skipped: boolean + readonly inspectedCount: number + readonly closedCount: number +} + +function parseFileDescriptor(entry: string): number | null { + const fd = Number(entry) + if (!Number.isSafeInteger(fd) || fd <= 2) { + return null + } + + return fd +} + +function isIgnoredFdScanError(error: unknown): boolean { + if (!(error instanceof Error) || !("code" in error)) { + return false + } + + const code = error.code + return typeof code === "string" && IGNORED_FD_ERROR_CODES.has(code) +} + +export function pruneLeakedDirectoryFileDescriptors( + dependencies: SpawnFdPrunerDependencies, + options: SpawnFdPruneOptions = {}, +): SpawnFdPruneResult { + if (dependencies.platform !== "darwin" || !dependencies.isBunRuntime) { + return { skipped: true, inspectedCount: 0, closedCount: 0 } + } + + const entries = dependencies.readdirSync(FD_DIRECTORY) + if (entries.length < (options.threshold ?? DEFAULT_PRUNE_THRESHOLD)) { + return { skipped: true, inspectedCount: 0, closedCount: 0 } + } + + let inspectedCount = 0 + let closedCount = 0 + for (const entry of entries) { + const fd = parseFileDescriptor(entry) + if (fd === null) { + continue + } + + try { + inspectedCount += 1 + if (dependencies.fstatSync(fd).isDirectory()) { + dependencies.closeSync(fd) + closedCount += 1 + } + } catch (error) { + if (isIgnoredFdScanError(error)) { + continue + } + + throw error + } + } + + return { skipped: false, inspectedCount, closedCount } +} + +export function pruneLeakedDirectoryFileDescriptorsBeforeSpawn(): SpawnFdPruneResult { + return pruneLeakedDirectoryFileDescriptors({ + platform: process.platform, + isBunRuntime: typeof Bun !== "undefined", + readdirSync, + fstatSync, + closeSync, + }) +} diff --git a/src/shared/tmux/runner.test.ts b/src/shared/tmux/runner.test.ts index 8b822086c..14a8ad7d6 100644 --- a/src/shared/tmux/runner.test.ts +++ b/src/shared/tmux/runner.test.ts @@ -12,6 +12,7 @@ const temporaryDirectories: string[] = [] const originalCmuxSocketPath = process.env.CMUX_SOCKET_PATH const originalTmux = process.env.TMUX const originalPath = process.env.PATH +const SHELL_BIN = process.platform === "win32" ? "sh" : "/bin/sh" async function createTemporaryDirectory(): Promise { const directoryPath = await fs.mkdtemp(path.join(os.tmpdir(), "tmux-runner-")) @@ -72,7 +73,7 @@ describe("runTmuxCommand", () => { try { // when - const result = await runTmuxCommand("sh", ["-c", "printf '%s\\n' real-tmux"]) + const result = await runTmuxCommand(SHELL_BIN, ["-c", "printf '%s\\n' real-tmux"]) // then expect(result).toEqual({ @@ -95,7 +96,7 @@ describe("runTmuxCommand", () => { const commandArguments = ["-c", "printf '%s\\n' '%42'"] // when - const result = await runTmuxCommand("sh", commandArguments) + const result = await runTmuxCommand(SHELL_BIN, commandArguments) // then expect(result).toEqual({ @@ -112,7 +113,7 @@ describe("runTmuxCommand", () => { const commandArguments = ["-c", "printf '%s\\n' 'some error' >&2; exit 1"] // when - const result = await runTmuxCommand("sh", commandArguments) + const result = await runTmuxCommand(SHELL_BIN, commandArguments) // then expect(result.success).toBe(false) @@ -127,7 +128,7 @@ describe("runTmuxCommand", () => { const commandScript = `counter_file="$1"; count=0; if [ -f "$counter_file" ]; then count=$(cat "$counter_file"); fi; count=$((count + 1)); printf '%s' "$count" > "$counter_file"; printf '%s\\n' 'temporary error' >&2; exit 1` // when - const result = await runTmuxCommand("sh", ["-c", commandScript, "sh", counterFilePath], { retry: 2 }) + const result = await runTmuxCommand(SHELL_BIN, ["-c", commandScript, "sh", counterFilePath], { retry: 2 }) // then expect(result.success).toBe(false) @@ -142,7 +143,7 @@ describe("runTmuxCommand", () => { const commandScript = `counter_file="$1"; count=0; if [ -f "$counter_file" ]; then count=$(cat "$counter_file"); fi; count=$((count + 1)); printf '%s' "$count" > "$counter_file"; printf '%s\\n' "can't find pane: %1" >&2; exit 1` // when - const result = await runTmuxCommand("sh", ["-c", commandScript, "sh", counterFilePath], { retry: 2 }) + const result = await runTmuxCommand(SHELL_BIN, ["-c", commandScript, "sh", counterFilePath], { retry: 2 }) // then expect(result.success).toBe(false) @@ -155,7 +156,7 @@ describe("runTmuxCommand", () => { const commandArguments = ["-c", "sleep 0.5"] // when - const result = await runTmuxCommand("sh", commandArguments, { timeoutMs: 50 }) + const result = await runTmuxCommand(SHELL_BIN, commandArguments, { timeoutMs: 50 }) // then expect(result.success).toBe(false) @@ -168,7 +169,7 @@ describe("runTmuxCommand", () => { const commandArguments = ["-c", "printf '%s\\n\\n' '%7'"] // when - const result = await runTmuxCommand("sh", commandArguments) + const result = await runTmuxCommand(SHELL_BIN, commandArguments) // then expect(result.output).toBe("%7") @@ -180,7 +181,7 @@ describe("runTmuxCommand", () => { const commandArguments = ["-c", "printf '%s\\n' '%9'"] // when - const { success, output } = await runTmuxCommand("sh", commandArguments) + const { success, output } = await runTmuxCommand(SHELL_BIN, commandArguments) // then expect(success).toBe(true) diff --git a/src/shared/tmux/runner.ts b/src/shared/tmux/runner.ts index 6bbd2cf4b..ec608bdb3 100644 --- a/src/shared/tmux/runner.ts +++ b/src/shared/tmux/runner.ts @@ -31,11 +31,15 @@ function isTerminalTmuxError(stderr: string): boolean { } function resolveTmuxExecutable(tmuxPath: string): string[] { + const executableName = tmuxPath.split(/[\\/]/).pop() if (!isCmuxCompatEnvironment()) { return [tmuxPath] } - const executableName = tmuxPath.split(/[\\/]/).pop() + if (executableName !== "tmux" && executableName !== "cmux") { + return [tmuxPath] + } + const cmuxExecutable = executableName === "cmux" ? tmuxPath : "cmux" return [cmuxExecutable, "__tmux-compat"] }