Skip to content
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
35 changes: 35 additions & 0 deletions tests/bounded-spawn-sync.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { expect, test } from "bun:test";
import {
describeCommand,
SyncSpawnTimeoutError,
spawnSyncBounded,
} from "./helpers/bounded-spawn-sync.ts";

test("a child that never finishes is killed at the bound and named in the error", () => {
const started = performance.now();
expect(() => spawnSyncBounded(["sleep", "30"], { timeout: 200 })).toThrow(
new SyncSpawnTimeoutError({ command: "sleep 30", timeoutMs: 200 })
);
expect(performance.now() - started).toBeLessThan(10_000);
});

test("a child stopped before it does any work is still killed at the bound", () => {
// Stand-in for a child that hangs before it starts (HACK-2247): it stops
// itself at once, and SIGKILL still ends a stopped process.
expect(() =>
spawnSyncBounded(["/bin/sh", "-c", "kill -STOP $$"], { timeout: 200 })
).toThrow(/^\/bin\/sh -c kill -STOP \$\$ did not finish within 200 ms/);
});

test("a finished child's result is returned unchanged, including a failure", () => {
const ok = spawnSyncBounded(["/bin/sh", "-c", "echo out; exit 3"]);
expect(ok.exitCode).toBe(3);
expect(ok.stdout.toString()).toBe("out\n");
});

test("long commands are shortened in the name", () => {
expect(describeCommand(["git", "commit", "-m", "message"])).toBe(
"git commit -m …"
);
expect(describeCommand(["ps"])).toBe("ps");
});
70 changes: 70 additions & 0 deletions tests/helpers/bounded-spawn-sync.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* Synchronous spawns for tests, always with a time bound (HACK-2247).
*
* A synchronous spawn blocks the event loop, so Bun's per-test timeout cannot
* fire while it waits. A child that never finishes, including one that hangs
* before it starts (seen on macOS when the system policy daemon stalled), then
* holds the whole test file forever. Every synchronous spawn in `tests/` and
* `scripts/check-*.ts` therefore passes a `timeout`; `tests/sync-spawn-bound.test.ts`
* enforces that for new call sites.
*
* Limit: the child is killed with SIGKILL at the bound and the spawn then
* waits for the kernel to reap it. A process the kernel cannot kill (stuck in
* an uninterruptible wait) can still hold the call.
*/

/** Default bound for one fixture command. Callers pass a smaller one when they can. */
export const SYNC_SPAWN_DEFAULT_TIMEOUT_MS = 30_000;

/** Error thrown when a bounded synchronous spawn hits its bound. */
export class SyncSpawnTimeoutError extends Error {
readonly command: string;
readonly timeoutMs: number;

constructor(opts: { readonly command: string; readonly timeoutMs: number }) {
super(
`${opts.command} did not finish within ${opts.timeoutMs} ms and was killed (a child that never starts or never exits; see HACK-2247).`
);
this.name = "SyncSpawnTimeoutError";
this.command = opts.command;
this.timeoutMs = opts.timeoutMs;
}
}

/** The command's name for messages: the executable and at most two arguments. */
export function describeCommand(cmd: readonly string[]): string {
const shown = cmd.slice(0, 3).join(" ");
return cmd.length > 3 ? `${shown} …` : shown;
}

/**
* `Bun.spawnSync` with a bound: kills the child with SIGKILL after `timeout`
* ms (default {@link SYNC_SPAWN_DEFAULT_TIMEOUT_MS}) and throws
* {@link SyncSpawnTimeoutError} naming the command. Any other result,
* including a non-zero exit, is returned unchanged for the caller to assert.
*/
export function spawnSyncBounded<
const In extends Bun.SpawnOptions.Writable = "ignore",
const Out extends Bun.SpawnOptions.Readable = "pipe",
const Err extends Bun.SpawnOptions.Readable = "pipe",
>(
cmd: readonly string[],
options: Omit<
Bun.SpawnOptions.SpawnSyncOptions<In, Out, Err>,
"killSignal"
> = {}
): Bun.SyncSubprocess<Out, Err> {
const timeoutMs = options.timeout ?? SYNC_SPAWN_DEFAULT_TIMEOUT_MS;
const result = Bun.spawnSync([...cmd], {
...options,
timeout: timeoutMs,
killSignal: "SIGKILL",
});
if (result.exitedDueToTimeout === true) {
throw new SyncSpawnTimeoutError({
command: describeCommand(cmd),
timeoutMs,
});
}
return result;
}
10 changes: 8 additions & 2 deletions tests/helpers/fixture-git.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { spawnSyncBounded } from "./bounded-spawn-sync.ts";

/**
* The one environment every test-fixture `git` command runs with (HACK-1459).
*
Expand Down Expand Up @@ -208,9 +210,13 @@ function spawnOptions(opts: FixtureGitOptions) {
} as const;
}

/** Runs one fixture `git` command synchronously. It does not throw on failure. */
/**
* Runs one fixture `git` command synchronously. It does not throw on failure,
* except when git does not finish within the bound (HACK-2247): then it throws
* naming the command.
*/
export function runFixtureGitSync(opts: FixtureGitOptions): FixtureGitResult {
const result = Bun.spawnSync(["git", ...opts.args], spawnOptions(opts));
const result = spawnSyncBounded(["git", ...opts.args], spawnOptions(opts));
return {
exitCode: result.exitCode,
stdout: result.stdout.toString(),
Expand Down
6 changes: 4 additions & 2 deletions tests/helpers/lifecycle-mux-fixture.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { chmod, readFile, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { spawnSyncBounded } from "./bounded-spawn-sync.ts";

/** A closed tmux transport stand-in. It starts the shipping wrapper/client, not
* a second controller. Only test-owned session metadata and captured children exist.
Expand Down Expand Up @@ -41,7 +42,7 @@ case "list-windows":
case "new-window": {
if(!state)process.exit(91);
const child=Bun.spawn(["/bin/sh","-c",args.at(-1)],{cwd:selected("-c"),detached:true,stdin:"ignore",stdout:"ignore",stderr:"ignore"});
child.unref();const started=Bun.spawnSync(["/bin/ps","-o","lstart=","-p",String(child.pid)],{env:{LC_ALL:"C",TZ:"UTC"}}).stdout.toString().trim();state.windows[selected("-n")]={pid:child.pid,started};await writeFile(file,JSON.stringify(state));break;
child.unref();const started=Bun.spawnSync(["/bin/ps","-o","lstart=","-p",String(child.pid)],{env:{LC_ALL:"C",TZ:"UTC"},timeout:10000,killSignal:"SIGKILL"}).stdout.toString().trim();state.windows[selected("-n")]={pid:child.pid,started};await writeFile(file,JSON.stringify(state));break;
}
case "list-panes": {
if(!state)process.exit(91);const name=selected("-t").split(":").at(-1);if(state.windows[name])console.log(state.windows[name].pid);break;
Expand All @@ -61,10 +62,11 @@ default:process.exit(93);

/** A process's start time as the stand-in records it; null when it has exited. */
function leaderStartedAt(pid: number): string | null {
const started = Bun.spawnSync(
const started = spawnSyncBounded(
["/bin/ps", "-o", "lstart=", "-p", String(pid)],
{
env: { LC_ALL: "C", TZ: "UTC" },
timeout: 10_000,
}
)
.stdout.toString()
Expand Down
3 changes: 2 additions & 1 deletion tests/helpers/secret-fixture.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { mkdir, writeFile } from "node:fs/promises";
import { join, resolve } from "node:path";
import { spawnSyncBounded } from "./bounded-spawn-sync.ts";
import { fixtureGitEnv } from "./fixture-git.ts";

/**
Expand All @@ -21,7 +22,7 @@ export function runFixtureGit(opts: {
readonly home: string;
readonly args: readonly string[];
}): string {
const result = Bun.spawnSync(["git", ...opts.args], {
const result = spawnSyncBounded(["git", ...opts.args], {
cwd: opts.cwd,
env: fixtureGitEnv({
env: {
Expand Down
3 changes: 2 additions & 1 deletion tests/nss-read-safety.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
} from "../src/lib/nss-read-safety.ts";
import { vettedHelperEnv } from "../src/lib/vetted-env.ts";
import internalModuleFixture from "./fixtures/nss-read/upstream-internal.pkcs11.json";
import { spawnSyncBounded } from "./helpers/bounded-spawn-sync.ts";
import { CURRENT_CA_PEM } from "./helpers/ca-certificates.ts";

const roots: string[] = [];
Expand Down Expand Up @@ -615,7 +616,7 @@ test("nonblocking strict reads reject a FIFO without a writer, and no-follow ref
}
const f = await fixture();
const pipe = join(f.root, "fifo-placeholder");
const created = Bun.spawnSync(["mkfifo", pipe]);
const created = spawnSyncBounded(["mkfifo", pipe], { timeout: 10_000 });
expect(created.exitCode).toBe(0);
const started = Date.now();
expect(await readTextOutcome(pipe, { maxBytes: 32 })).toBe("unreadable");
Expand Down
20 changes: 13 additions & 7 deletions tests/python-free-bootstrap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import { renderBootstrap } from "../scripts/installer/bootstrap.ts";
import { hash } from "../scripts/installer/integrity.ts";
import { spawnSyncBounded } from "./helpers/bounded-spawn-sync.ts";

// Filesystem fixtures may span several bounded subprocesses; the suite default is only 5 s.
// Every spawn here is synchronous, so the per-test timeout cannot interrupt one:
// each carries its own bound and fails naming the command (HACK-2247).
const fixtureTest = (name: string, run: () => void | Promise<void>) =>
test(name, run, 30_000);
const roots: string[] = [];
Expand Down Expand Up @@ -102,7 +105,7 @@ for (const shell of [
return;
}
const f = fixture();
const result = Bun.spawnSync([...shell, f.bootstrap], {
const result = spawnSyncBounded([...shell, f.bootstrap], {
env: f.env,
timeout: 10_000,
stdout: "pipe",
Expand Down Expand Up @@ -134,7 +137,10 @@ for (const shell of [
expect(Bun.which("python3", { PATH: f.bin })).toBeNull();
expect(Bun.which("bun", { PATH: f.bin })).toBeNull();
expect(existsSync(`${f.home}/.hack-next`)).toBe(false); // Stand-in records protocol; compiled manager has a separate test.
expect(Bun.spawnSync(["/bin/sh", "-n", f.bootstrap]).exitCode).toBe(0);
expect(
spawnSyncBounded(["/bin/sh", "-n", f.bootstrap], { timeout: 10_000 })
.exitCode
).toBe(0);
}
);
}
Expand Down Expand Up @@ -165,7 +171,7 @@ for (const [name, options, error] of [
const f = fixture(options);
const sentinel = join(f.home, "keep");
writeFileSync(sentinel, "original");
const result = Bun.spawnSync(["/bin/sh", f.bootstrap], {
const result = spawnSyncBounded(["/bin/sh", f.bootstrap], {
env: f.env,
timeout: 10_000,
stdout: "pipe",
Expand All @@ -183,14 +189,14 @@ fixtureTest(
"bootstrap refuses relative roots and missing tools before network requests",
() => {
const f = fixture();
const relative = Bun.spawnSync(
const relative = spawnSyncBounded(
["/bin/sh", f.bootstrap, "--root", "relative"],
{ env: f.env, timeout: 10_000 }
);
expect(relative.exitCode).not.toBe(0);
expect(relative.stderr.toString()).toContain("absolute");
rmSync(join(f.bin, "shasum"));
const missing = Bun.spawnSync(["/bin/sh", f.bootstrap], {
const missing = spawnSyncBounded(["/bin/sh", f.bootstrap], {
env: f.env,
timeout: 10_000,
});
Expand Down Expand Up @@ -220,7 +226,7 @@ fixtureTest(
["--archive", ""],
]) {
const f = fixture();
const result = Bun.spawnSync(["/bin/sh", f.bootstrap, ...args], {
const result = spawnSyncBounded(["/bin/sh", f.bootstrap, ...args], {
env: f.env,
timeout: 10_000,
});
Expand All @@ -235,7 +241,7 @@ fixtureTest(
() => {
const f = fixture();
const checksum = "a".repeat(64);
const result = Bun.spawnSync(
const result = spawnSyncBounded(
[
"/bin/sh",
f.bootstrap,
Expand Down
Loading
Loading