fix(shared): prune Bun directory fds before spawn
This commit is contained in:
@@ -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
|
||||
)
|
||||
|
||||
@@ -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<string, unknown>
|
||||
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) {
|
||||
|
||||
@@ -79,7 +79,7 @@ export async function executeHookCommand(
|
||||
|
||||
const proc = spawn(finalCommand, {
|
||||
cwd,
|
||||
shell: true,
|
||||
shell: isWin32 ? true : "/bin/sh",
|
||||
detached: !isWin32,
|
||||
env,
|
||||
});
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
})
|
||||
@@ -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,
|
||||
})
|
||||
}
|
||||
@@ -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<string> {
|
||||
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)
|
||||
|
||||
@@ -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"]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user