From 22caff17d6e66ba1f7960effdb98825ff399de5b Mon Sep 17 00:00:00 2001 From: Antonio Viggiano Date: Mon, 26 Oct 2026 19:40:05 +0000 Subject: [PATCH 1/5] fix(runtime): reclaim test scratch after each case --- .../scripts/run-runtime-test-shard.mjs | 9 +- packages/runtime/scripts/run-tests.mjs | 9 +- packages/runtime/scripts/test-scratch.mjs | 135 ++++++++++++++++++ packages/runtime/test/runtime-test-shard.ts | 11 +- packages/runtime/test/runtime.test.ts | 3 +- packages/runtime/test/temporary-root.test.ts | 57 ++++++++ packages/runtime/test/temporary-root.ts | 105 +++++++++----- 7 files changed, 278 insertions(+), 51 deletions(-) create mode 100644 packages/runtime/scripts/test-scratch.mjs create mode 100644 packages/runtime/test/temporary-root.test.ts diff --git a/packages/runtime/scripts/run-runtime-test-shard.mjs b/packages/runtime/scripts/run-runtime-test-shard.mjs index 566c9765b..f7505a0d5 100644 --- a/packages/runtime/scripts/run-runtime-test-shard.mjs +++ b/packages/runtime/scripts/run-runtime-test-shard.mjs @@ -1,4 +1,4 @@ -import { spawnSync } from "node:child_process"; +import { runTests } from "./test-scratch.mjs"; const args = process.argv.slice(2).filter((argument) => argument !== "--"); const shard = args[0]; @@ -6,9 +6,6 @@ if (args.length !== 1 || !/^[1-9][0-9]*\/[1-9][0-9]*$/u.test(shard ?? "")) { throw new Error("run-runtime-test-shard.mjs requires exactly one index/total argument"); } -const result = spawnSync(process.execPath, ["--test", "dist-test/test/runtime.test.js"], { - env: { ...process.env, ULTRAFUZZ_RUNTIME_TEST_SHARD: shard }, - stdio: "inherit" +process.exitCode = await runTests(["--test", "dist-test/test/runtime.test.js"], { + ULTRAFUZZ_RUNTIME_TEST_SHARD: shard }); - -process.exit(result.status ?? 1); diff --git a/packages/runtime/scripts/run-tests.mjs b/packages/runtime/scripts/run-tests.mjs index 8eb8d5b61..8d04cd93b 100644 --- a/packages/runtime/scripts/run-tests.mjs +++ b/packages/runtime/scripts/run-tests.mjs @@ -1,7 +1,8 @@ import { readdirSync } from "node:fs"; -import { spawnSync } from "node:child_process"; import path from "node:path"; +import { runTests } from "./test-scratch.mjs"; + const selectors = process.argv.slice(2).filter((argument) => !argument.startsWith("-")); const passThrough = process.argv.slice(2).filter((argument) => argument.startsWith("-")); @@ -23,8 +24,4 @@ const testFiles = .map((entry) => path.join("dist-test/test", entry)) : selectors.flatMap((selector) => selectorFiles.get(selector) ?? [`dist-test/test/${selector}.test.js`]); -const result = spawnSync("node", ["--test", ...testFiles, ...passThrough], { - stdio: "inherit" -}); - -process.exit(result.status ?? 1); +process.exitCode = await runTests(["--test", ...passThrough, ...testFiles]); diff --git a/packages/runtime/scripts/test-scratch.mjs b/packages/runtime/scripts/test-scratch.mjs new file mode 100644 index 000000000..ed57a75a2 --- /dev/null +++ b/packages/runtime/scripts/test-scratch.mjs @@ -0,0 +1,135 @@ +import { spawn } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +function restoreWritePermissions(root) { + const stat = fs.lstatSync(root); + if (stat.isSymbolicLink() || (!stat.isDirectory() && !stat.isFile())) return; + fs.chmodSync(root, stat.mode | (stat.isDirectory() ? 0o700 : 0o600)); + if (!stat.isDirectory()) return; + for (const entry of fs.readdirSync(root, { withFileTypes: true })) { + if (entry.isDirectory() || entry.isFile()) restoreWritePermissions(path.join(root, entry.name)); + } +} + +function removeScratch(root) { + restoreWritePermissions(root); + fs.rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +} + +function pidIsAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code !== "ESRCH"; + } +} + +function activeTestChild(root) { + let marker; + try { + marker = fs.readFileSync(path.join(root, ".test-child-pid"), "utf8"); + } catch (error) { + if (error.code === "ENOENT") return false; + throw error; + } + const pid = Number(marker.trim()); + return Number.isSafeInteger(pid) && pid > 0 && pidIsAlive(pid); +} + +function ownedScratchDirectory() { + const parent = path.join(fs.realpathSync(os.tmpdir()), `ultrafuzz-runtime-tests-${process.getuid?.() ?? "user"}`); + fs.mkdirSync(parent, { mode: 0o700, recursive: true }); + const parentStat = fs.lstatSync(parent); + if ( + !parentStat.isDirectory() || + (process.getuid !== undefined && parentStat.uid !== process.getuid()) || + (parentStat.mode & 0o077) !== 0 + ) { + throw new Error(`Runtime test scratch parent is not owned by this process: ${parent}`); + } + for (const entry of fs.readdirSync(parent, { withFileTypes: true })) { + const match = /^(?:run|reap)-([1-9][0-9]*)-[A-Za-z0-9]+$/u.exec(entry.name); + if (!match || !entry.isDirectory() || pidIsAlive(Number(match[1]))) continue; + const stale = path.join(parent, entry.name); + if (activeTestChild(stale)) continue; + const claimed = path.join(parent, `reap-${process.pid}-${randomUUID().replaceAll("-", "")}`); + try { + fs.renameSync(stale, claimed); + } catch (error) { + if (error.code === "ENOENT") continue; // Another test launcher claimed it first. + throw error; + } + removeScratch(claimed); + } + return fs.mkdtempSync(path.join(parent, `run-${process.pid}-`)); +} + +export async function runTests(args, extraEnv = {}) { + let scratch; + try { + scratch = ownedScratchDirectory(); + } catch (error) { + process.stderr.write( + `Runtime test scratch setup failed${error.code === "ENOSPC" ? " (disk full, ENOSPC)" : ""}: ${String(error)}\n` + ); + return 1; + } + + let status; + let diskFull = false; + try { + const child = spawn(process.execPath, args, { + env: { ...process.env, ...extraEnv, TMPDIR: scratch, TMP: scratch, TEMP: scratch }, + stdio: ["inherit", "pipe", "pipe"] + }); + let markerError; + if (child.pid !== undefined) { + try { + fs.writeFileSync(path.join(scratch, ".test-child-pid"), `${child.pid}\n`, { mode: 0o600 }); + } catch (error) { + markerError = error; + child.kill("SIGKILL"); + } + } + for (const [stream, output] of [ + [child.stdout, process.stdout], + [child.stderr, process.stderr] + ]) { + let tail = ""; + stream.on("data", (chunk) => { + const text = tail + String(chunk); + if (text.includes("ENOSPC")) diskFull = true; + tail = text.slice(-5); + }); + stream.pipe(output, { end: false }); + } + status = await new Promise((resolve) => { + child.once("error", (error) => { + if (error.code === "ENOSPC") diskFull = true; + process.stderr.write(`Runtime test process could not start: ${String(error)}\n`); + resolve(1); + }); + child.once("close", (code) => resolve(code ?? 1)); + }); + if (markerError !== undefined) { + process.stderr.write(`Runtime test scratch owner could not be recorded: ${String(markerError)}\n`); + status = 1; + } + if (diskFull) { + process.stderr.write(`Runtime tests exhausted scratch space (ENOSPC) under ${scratch}.\n`); + status = 1; + } + } finally { + try { + removeScratch(scratch); + } catch (error) { + process.stderr.write(`Runtime test scratch cleanup failed for ${scratch}: ${String(error)}\n`); + status = 1; + } + } + return status; +} diff --git a/packages/runtime/test/runtime-test-shard.ts b/packages/runtime/test/runtime-test-shard.ts index c3c72db33..788d9c49c 100644 --- a/packages/runtime/test/runtime-test-shard.ts +++ b/packages/runtime/test/runtime-test-shard.ts @@ -1,5 +1,8 @@ import crypto from "node:crypto"; import nodeTest from "node:test"; +import type { TestContext } from "node:test"; + +import { withTemporaryRootScope } from "./temporary-root.js"; const RUNTIME_TEST_SHARD_ENV = "ULTRAFUZZ_RUNTIME_TEST_SHARD"; const MAX_RUNTIME_TEST_SHARDS = 32; @@ -39,7 +42,13 @@ const selectedShard = parseRuntimeTestShard(process.env[RUNTIME_TEST_SHARD_ENV]) */ export const test = ((name: string, ...args: unknown[]) => { if (selectedShard === undefined || runtimeTestShardForName(name, selectedShard.total) === selectedShard.index) { - return Reflect.apply(nodeTest, undefined, [name, ...args]); + const bodyIndex = typeof args[0] === "function" ? 0 : 1; + const body = args[bodyIndex] as (context: TestContext) => void | Promise; + return Reflect.apply(nodeTest, undefined, [ + name, + ...args.slice(0, bodyIndex), + (context: TestContext) => withTemporaryRootScope(context, () => body(context)) + ]); } const reason = `assigned to runtime test shard ${runtimeTestShardForName(name, selectedShard.total)}/${selectedShard.total}`; diff --git a/packages/runtime/test/runtime.test.ts b/packages/runtime/test/runtime.test.ts index 9d6481716..de9b302f2 100644 --- a/packages/runtime/test/runtime.test.ts +++ b/packages/runtime/test/runtime.test.ts @@ -698,7 +698,8 @@ function loadOpenRouterPatchedSmithersUrl(): Promise { const patch = SMITHERS_COMPATIBILITY_PATCHES.find((candidate) => candidate.id === "agents_ordered_stdout"); assert.ok(patch); const pinnedRoot = path.dirname(pinnedRunnerSourceDir("@smthrs/agents", "BaseCliAgent/BaseCliAgent")); - const isolatedRoot = path.join(tempProject(), "agents"); + // This imported module is cached and reused by later test cases. + const isolatedRoot = path.join(temporaryRoot("ufz-runtime-", "process"), "agents"); fs.cpSync(pinnedRoot, isolatedRoot, { recursive: true }); fs.rmSync(path.join(isolatedRoot, "node_modules"), { recursive: true, force: true }); fs.symlinkSync(path.dirname(path.dirname(pinnedRoot)), path.join(isolatedRoot, "node_modules"), "dir"); diff --git a/packages/runtime/test/temporary-root.test.ts b/packages/runtime/test/temporary-root.test.ts new file mode 100644 index 000000000..b886d4b38 --- /dev/null +++ b/packages/runtime/test/temporary-root.test.ts @@ -0,0 +1,57 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; + +import { test } from "./runtime-test-shard.js"; +import { registerTemporaryPath, temporaryRoot } from "./temporary-root.js"; + +let sealedRoot: string; +let adjacentRoot: string; +let sharedRoot: string; + +test("test-scoped scratch includes sealed snapshots and registered adjacent paths", () => { + sealedRoot = temporaryRoot("ufz-test-cleanup-"); + const snapshot = path.join(sealedRoot, "snapshot"); + fs.mkdirSync(snapshot); + fs.writeFileSync(path.join(snapshot, "module.js"), "export {};\n"); + fs.chmodSync(path.join(snapshot, "module.js"), 0o400); + fs.chmodSync(snapshot, 0o500); + + adjacentRoot = registerTemporaryPath(fs.mkdtempSync(path.join(path.dirname(sealedRoot), "ufz-test-adjacent-"))); + fs.writeFileSync(path.join(adjacentRoot, "data"), "fixture"); + fs.chmodSync(adjacentRoot, 0o500); + + sharedRoot = temporaryRoot("ufz-test-shared-", "process"); + assert.equal(fs.existsSync(sealedRoot), true); +}); + +test("scratch is reclaimed between test cases while a shared fixture remains", () => { + assert.equal(fs.existsSync(sealedRoot), false); + assert.equal(fs.existsSync(adjacentRoot), false); + assert.equal(fs.existsSync(sharedRoot), true); +}); + +test("scratch is reclaimed after a failed test body", () => { + const fixture = temporaryRoot("ufz-failed-test-"); + const marker = path.join(fixture, "created-root.txt"); + const childTest = path.join(fixture, "failure.test.mjs"); + fs.writeFileSync( + childTest, + [ + `import { test } from ${JSON.stringify(new URL("./runtime-test-shard.js", import.meta.url).href)};`, + `import { temporaryRoot } from ${JSON.stringify(new URL("./temporary-root.js", import.meta.url).href)};`, + 'import fs from "node:fs";', + 'test("deliberate failure", () => {', + ` fs.writeFileSync(${JSON.stringify(marker)}, temporaryRoot("ufz-failing-body-"));`, + ' throw new Error("deliberate failure sentinel");', + "});" + ].join("\n") + ); + const child = spawnSync(process.execPath, process.versions.bun === undefined ? [childTest] : ["test", childTest], { + encoding: "utf8" + }); + assert.equal(child.status, 1, `${child.stdout}${child.stderr}`); + assert.match(`${child.stdout}${child.stderr}`, /deliberate failure sentinel/u); + assert.equal(fs.existsSync(fs.readFileSync(marker, "utf8")), false); +}); diff --git a/packages/runtime/test/temporary-root.ts b/packages/runtime/test/temporary-root.ts index 29f88419d..54d3b24d4 100644 --- a/packages/runtime/test/temporary-root.ts +++ b/packages/runtime/test/temporary-root.ts @@ -1,9 +1,11 @@ +import { AsyncLocalStorage } from "node:async_hooks"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import type { TestContext } from "node:test"; /** - * Create a temporary directory for a test and remove it when the process exits. + * Create a temporary directory for a test and remove it after that test. * * Two problems are solved here that every fixture otherwise has to remember. * @@ -17,20 +19,41 @@ import path from "node:path"; * The root is removable. Sealed execution snapshots are left `dr-x------`, and * unlinking an entry needs the write bit on its parent rather than on the entry * itself, so `rmSync` cannot remove a tree that contains one. Owner write - * permission is restored on directories on the way down, which is the same + * permission is restored on files and directories on the way down, which is the same * treatment `restoreRemovableDirectoryPermissions` applies in `clean.ts`. */ -const registered: string[] = []; +const registered = new Set(); +const testRoots = new AsyncLocalStorage(); let exitHookInstalled = false; -export function temporaryRoot(prefix: string): string { - const root = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), prefix)); - registered.push(root); +/** Attach roots created by a test body to its after hook, including on assertion failure. */ +export function withTemporaryRootScope(context: TestContext, body: () => void | Promise): void | Promise { + const roots: string[] = []; + context.after(() => removeRoots(roots)); + return testRoots.run(roots, body); +} + +export function temporaryRoot(prefix: string, lifetime: "test" | "process" = "test"): string { + let root: string; + try { + root = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), prefix)); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOSPC") { + throw new Error(`Runtime test scratch space is full (ENOSPC) while creating ${prefix} in ${os.tmpdir()}`, { + cause: error + }); + } + throw error; + } + registerTemporaryPath(root, lifetime); + return root; +} + +function installExitHook(): void { if (!exitHookInstalled) { exitHookInstalled = true; process.on("exit", removeRegisteredRoots); } - return root; } /** @@ -47,49 +70,57 @@ export function privateHomeEnv(): { HOME: string } { * rest. Fixtures that place a directory beside their root rather than inside it * are otherwise invisible to this module and survive the run. */ -export function registerTemporaryPath(directory: string): string { - registered.push(directory); - if (!exitHookInstalled) { - exitHookInstalled = true; - process.on("exit", removeRegisteredRoots); - } +export function registerTemporaryPath(directory: string, lifetime: "test" | "process" = "test"): string { + registered.add(directory); + if (lifetime === "test") testRoots.getStore()?.push(directory); + installExitHook(); return directory; } -/** Remove every registered root. */ +function removeRoots(roots: string[]): void { + const errors: Error[] = []; + for (let root = roots.pop(); root !== undefined; root = roots.pop()) { + try { + removeTemporaryRoot(root); + registered.delete(root); + } catch (error) { + errors.push(error as Error); + } + } + if (errors.length > 0) throw new AggregateError(errors, "Failed to remove runtime test scratch directories"); +} + +/** Last resort for module-scoped fixtures and interrupted test cleanup. */ function removeRegisteredRoots(): void { - for (let root = registered.pop(); root !== undefined; root = registered.pop()) removeTemporaryRoot(root); + for (const root of registered) { + try { + removeTemporaryRoot(root); + } catch (error) { + process.stderr.write(`Failed to remove runtime test scratch directory ${root}: ${String(error)}\n`); + process.exitCode = 1; + } + } } function removeTemporaryRoot(root: string): void { - restoreRemovableDirectoryPermissions(root); - try { - fs.rmSync(root, { recursive: true, force: true }); - } catch { - // A test that deliberately locks a tree down is not worth failing the run. - } + restoreRemovablePermissions(root); + fs.rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); } -function restoreRemovableDirectoryPermissions(root: string): void { +function restoreRemovablePermissions(root: string): void { let stat: fs.Stats; try { stat = fs.lstatSync(root); - } catch { - return; - } - if (stat.isSymbolicLink() || !stat.isDirectory()) return; - try { - fs.chmodSync(root, stat.mode | 0o700); - } catch { - // Best effort: rmSync reports the actionable failure. - } - let entries: fs.Dirent[]; - try { - entries = fs.readdirSync(root, { withFileTypes: true }); - } catch { - return; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return; + throw error; } + if (stat.isSymbolicLink()) return; + if (!stat.isDirectory() && !stat.isFile()) return; + fs.chmodSync(root, stat.mode | (stat.isDirectory() ? 0o700 : 0o600)); + if (!stat.isDirectory()) return; + const entries = fs.readdirSync(root, { withFileTypes: true }); for (const entry of entries) { - if (entry.isDirectory()) restoreRemovableDirectoryPermissions(path.join(root, entry.name)); + if (entry.isDirectory() || entry.isFile()) restoreRemovablePermissions(path.join(root, entry.name)); } } From fed4f75e2a07de6b7a13cd0fd6153cd979d1adf1 Mon Sep 17 00:00:00 2001 From: Antonio Viggiano Date: Mon, 26 Oct 2026 19:54:54 +0000 Subject: [PATCH 2/5] test(runtime): verify failed-case cleanup before exit --- packages/runtime/test/temporary-root.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/runtime/test/temporary-root.test.ts b/packages/runtime/test/temporary-root.test.ts index b886d4b38..39c96ce10 100644 --- a/packages/runtime/test/temporary-root.test.ts +++ b/packages/runtime/test/temporary-root.test.ts @@ -35,16 +35,22 @@ test("scratch is reclaimed between test cases while a shared fixture remains", ( test("scratch is reclaimed after a failed test body", () => { const fixture = temporaryRoot("ufz-failed-test-"); const marker = path.join(fixture, "created-root.txt"); + const verified = path.join(fixture, "verified.txt"); const childTest = path.join(fixture, "failure.test.mjs"); fs.writeFileSync( childTest, [ + 'import assert from "node:assert/strict";', `import { test } from ${JSON.stringify(new URL("./runtime-test-shard.js", import.meta.url).href)};`, `import { temporaryRoot } from ${JSON.stringify(new URL("./temporary-root.js", import.meta.url).href)};`, 'import fs from "node:fs";', 'test("deliberate failure", () => {', ` fs.writeFileSync(${JSON.stringify(marker)}, temporaryRoot("ufz-failing-body-"));`, ' throw new Error("deliberate failure sentinel");', + "});", + 'test("cleanup follows failed test", () => {', + ` assert.equal(fs.existsSync(fs.readFileSync(${JSON.stringify(marker)}, "utf8")), false);`, + ` fs.writeFileSync(${JSON.stringify(verified)}, "yes");`, "});" ].join("\n") ); @@ -53,5 +59,6 @@ test("scratch is reclaimed after a failed test body", () => { }); assert.equal(child.status, 1, `${child.stdout}${child.stderr}`); assert.match(`${child.stdout}${child.stderr}`, /deliberate failure sentinel/u); + assert.equal(fs.readFileSync(verified, "utf8"), "yes", `${child.stdout}${child.stderr}`); assert.equal(fs.existsSync(fs.readFileSync(marker, "utf8")), false); }); From 1dd4f918ff654926bca306ef12f519c700ee00d4 Mon Sep 17 00:00:00 2001 From: Antonio Viggiano Date: Mon, 26 Oct 2026 20:19:40 +0000 Subject: [PATCH 3/5] fix(runtime): retain scratch while test descendants may be active --- packages/runtime/scripts/test-scratch.mjs | 255 ++++++++++++++++-- packages/runtime/test/temporary-root.test.ts | 18 ++ packages/runtime/test/temporary-root.ts | 30 ++- packages/runtime/test/test-scratch.test.ts | 265 +++++++++++++++++++ 4 files changed, 532 insertions(+), 36 deletions(-) create mode 100644 packages/runtime/test/test-scratch.test.ts diff --git a/packages/runtime/scripts/test-scratch.mjs b/packages/runtime/scripts/test-scratch.mjs index ed57a75a2..a14465a95 100644 --- a/packages/runtime/scripts/test-scratch.mjs +++ b/packages/runtime/scripts/test-scratch.mjs @@ -6,11 +6,12 @@ import path from "node:path"; function restoreWritePermissions(root) { const stat = fs.lstatSync(root); - if (stat.isSymbolicLink() || (!stat.isDirectory() && !stat.isFile())) return; - fs.chmodSync(root, stat.mode | (stat.isDirectory() ? 0o700 : 0o600)); + // Unlinking a file needs write permission on its parent, not on the file. + // A file may be hard-linked outside scratch, so never change its mode. if (!stat.isDirectory()) return; + fs.chmodSync(root, stat.mode | 0o700); for (const entry of fs.readdirSync(root, { withFileTypes: true })) { - if (entry.isDirectory() || entry.isFile()) restoreWritePermissions(path.join(root, entry.name)); + if (entry.isDirectory()) restoreWritePermissions(path.join(root, entry.name)); } } @@ -28,16 +29,162 @@ function pidIsAlive(pid) { } } -function activeTestChild(root) { +function processGroupIsAlive(pgid) { + // A Node --test runner can exit while its per-file workers keep running. + // On Windows a group probe is unavailable, so stale scratch is retained. + return process.platform === "win32" || pidIsAlive(-pgid); +} + +function recordedTestGroupIsDead(root) { let marker; try { - marker = fs.readFileSync(path.join(root, ".test-child-pid"), "utf8"); + marker = fs.readFileSync(path.join(root, ".test-child-pgid"), "utf8"); } catch (error) { - if (error.code === "ENOENT") return false; + if (error.code === "ENOENT") return false; // The launcher may have died before publishing the marker. throw error; } - const pid = Number(marker.trim()); - return Number.isSafeInteger(pid) && pid > 0 && pidIsAlive(pid); + if (!/^[1-9][0-9]*\n?$/u.test(marker)) return false; + const pgid = Number(marker.trim()); + return Number.isSafeInteger(pgid) && !processGroupIsAlive(pgid); +} + +function scratchExhaustionError(error) { + return error.code === "ENOSPC" || error.code === "EDQUOT" || error.errno === -122; +} + +const scratchExhaustionOutput = /\b(?:ENOSPC|EDQUOT)\b|Unknown system error:?\s*-122\b|\berrno\s*[:=]?\s*-122\b/iu; + +function pathIsWithin(candidate, root) { + return candidate === root || candidate.startsWith(`${root}${path.sep}`); +} + +function processStartTicks(pid) { + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + const commandEnd = stat.lastIndexOf(") "); + if (commandEnd === -1) return undefined; + // Field 22 is starttime; the text after the command starts at field 3. + const value = stat + .slice(commandEnd + 2) + .trim() + .split(/\s+/u)[19]; + return /^[0-9]+$/u.test(value ?? "") ? BigInt(value) : undefined; + } catch { + return undefined; + } +} + +function recordedLauncherStart(root) { + let marker; + try { + marker = fs.readFileSync(path.join(root, ".test-launcher-start"), "utf8"); + } catch { + return undefined; + } + return /^[0-9]+\n$/u.test(marker) ? BigInt(marker.trim()) : undefined; +} + +function scratchUseStatus(scratch, original = scratch) { + if (process.platform !== "linux" || process.getuid === undefined) return "uncertain"; + const launcherStart = recordedLauncherStart(scratch); + if (launcherStart === undefined) return "uncertain"; + const paths = [original, scratch]; + let processes; + try { + processes = fs.readdirSync("/proc", { withFileTypes: true }); + } catch { + return "uncertain"; + } + let uncertain = false; + for (const entry of processes) { + if (!/^[0-9]+$/u.test(entry.name)) continue; + const pid = Number(entry.name); + const processRoot = path.join("/proc", entry.name); + const started = processStartTicks(entry.name); + // A process that started before this launcher predates its scratch. + // Readable cwd/fds may still show later use; age excuses only read errors. + const olderProcess = started !== undefined && started < launcherStart; + let readUncertain = false; + try { + // Non-dumpable processes can have root-owned /proc directories even + // while their real/effective UIDs still match this test launcher. + const status = fs.readFileSync(path.join(processRoot, "status"), "utf8"); + const match = /^Uid:\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)/mu.exec(status); + if (match === null) readUncertain = true; + else if (!match.slice(1).some((uid) => Number(uid) === process.getuid())) continue; + } catch (error) { + if ((error.code === "ENOENT" || error.code === "ESRCH") && !pidIsAlive(pid)) continue; + readUncertain = true; + } + let environment; + try { + environment = fs.readFileSync(path.join(processRoot, "environ"), "utf8"); + } catch (error) { + if ((error.code === "ENOENT" || error.code === "ESRCH") && !pidIsAlive(pid)) continue; + readUncertain = true; + } + if ( + environment + ?.split("\0") + .some((field) => + paths.some( + (directory) => + field === `TMPDIR=${directory}` || field === `TMP=${directory}` || field === `TEMP=${directory}` + ) + ) + ) { + return "active"; + } + try { + const cwd = fs.readlinkSync(path.join(processRoot, "cwd")); + if (paths.some((directory) => pathIsWithin(cwd, directory))) return "active"; + for (const descriptor of fs.readdirSync(path.join(processRoot, "fd"))) { + let target; + try { + target = fs.readlinkSync(path.join(processRoot, "fd", descriptor)); + } catch (error) { + if (error.code === "ENOENT" && !pidIsAlive(pid)) break; + if (error.code === "ENOENT") continue; // A descriptor closed during the scan. + throw error; + } + if (paths.some((directory) => pathIsWithin(target, directory))) return "active"; + } + } catch (error) { + if ((error.code === "ENOENT" || error.code === "ESRCH") && !pidIsAlive(pid)) continue; + readUncertain = true; + } + if (readUncertain) { + if (!pidIsAlive(pid)) continue; + const currentStart = processStartTicks(entry.name); + const stillOlder = currentStart !== undefined && currentStart < launcherStart; + if (!olderProcess || !stillOlder) uncertain = true; + } + } + return uncertain ? "uncertain" : "clear"; +} + +function signalTestGroup(pgid, signal) { + try { + process.kill(-pgid, signal); + } catch (error) { + if (error.code !== "ESRCH") throw error; + } +} + +function recordedOriginalScratch(root, parent) { + let original; + try { + original = fs.readFileSync(path.join(root, ".test-scratch-origin"), "utf8"); + } catch (error) { + if (error.code === "ENOENT") return undefined; + throw error; + } + if (!original.endsWith("\n")) return undefined; + original = original.slice(0, -1); + if (path.dirname(original) !== parent || !/^run-[1-9][0-9]*-[A-Za-z0-9]+$/u.test(path.basename(original))) { + return undefined; + } + return original; } function ownedScratchDirectory() { @@ -55,7 +202,9 @@ function ownedScratchDirectory() { const match = /^(?:run|reap)-([1-9][0-9]*)-[A-Za-z0-9]+$/u.exec(entry.name); if (!match || !entry.isDirectory() || pidIsAlive(Number(match[1]))) continue; const stale = path.join(parent, entry.name); - if (activeTestChild(stale)) continue; + const original = recordedOriginalScratch(stale, parent); + if (original === undefined || !recordedTestGroupIsDead(stale) || scratchUseStatus(stale, original) !== "clear") + continue; const claimed = path.join(parent, `reap-${process.pid}-${randomUUID().replaceAll("-", "")}`); try { fs.renameSync(stale, claimed); @@ -63,9 +212,18 @@ function ownedScratchDirectory() { if (error.code === "ENOENT") continue; // Another test launcher claimed it first. throw error; } - removeScratch(claimed); + // A process can start while we claim the directory, and an escaped child + // still has the original run path in its environment after the rename. + if (recordedTestGroupIsDead(claimed) && scratchUseStatus(claimed, original) === "clear") removeScratch(claimed); + } + const scratch = fs.mkdtempSync(path.join(parent, `run-${process.pid}-`)); + fs.writeFileSync(path.join(scratch, ".test-scratch-origin"), `${scratch}\n`, { mode: 0o600, flag: "wx" }); + if (process.platform === "linux") { + const start = processStartTicks("self"); + if (start === undefined) throw new Error("Runtime test launcher start time is unavailable from /proc/self/stat"); + fs.writeFileSync(path.join(scratch, ".test-launcher-start"), `${start}\n`, { mode: 0o600, flag: "wx" }); } - return fs.mkdtempSync(path.join(parent, `run-${process.pid}-`)); + return scratch; } export async function runTests(args, extraEnv = {}) { @@ -74,25 +232,41 @@ export async function runTests(args, extraEnv = {}) { scratch = ownedScratchDirectory(); } catch (error) { process.stderr.write( - `Runtime test scratch setup failed${error.code === "ENOSPC" ? " (disk full, ENOSPC)" : ""}: ${String(error)}\n` + `Runtime test scratch setup failed${scratchExhaustionError(error) ? " (disk full or quota exceeded, ENOSPC/EDQUOT)" : ""}: ${String(error)}\n` ); return 1; } let status; let diskFull = false; + let childGroupId; + let childClosed = false; + let childExitCode; + const signalHandlers = []; try { const child = spawn(process.execPath, args, { env: { ...process.env, ...extraEnv, TMPDIR: scratch, TMP: scratch, TEMP: scratch }, - stdio: ["inherit", "pipe", "pipe"] + stdio: ["inherit", "pipe", "pipe"], + detached: process.platform !== "win32" }); + childGroupId = child.pid; + if (process.platform !== "win32" && childGroupId !== undefined) { + for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) { + const handler = () => signalTestGroup(childGroupId, signal); + process.on(signal, handler); + signalHandlers.push([signal, handler]); + } + } let markerError; if (child.pid !== undefined) { try { - fs.writeFileSync(path.join(scratch, ".test-child-pid"), `${child.pid}\n`, { mode: 0o600 }); + const pendingMarker = path.join(scratch, ".test-child-pgid.pending"); + fs.writeFileSync(pendingMarker, `${child.pid}\n`, { mode: 0o600, flag: "wx" }); + fs.renameSync(pendingMarker, path.join(scratch, ".test-child-pgid")); } catch (error) { markerError = error; - child.kill("SIGKILL"); + if (process.platform === "win32") child.kill("SIGKILL"); + else signalTestGroup(child.pid, "SIGKILL"); } } for (const [stream, output] of [ @@ -102,33 +276,66 @@ export async function runTests(args, extraEnv = {}) { let tail = ""; stream.on("data", (chunk) => { const text = tail + String(chunk); - if (text.includes("ENOSPC")) diskFull = true; - tail = text.slice(-5); + if (scratchExhaustionOutput.test(text)) diskFull = true; + tail = text.slice(-40); }); stream.pipe(output, { end: false }); } status = await new Promise((resolve) => { child.once("error", (error) => { - if (error.code === "ENOSPC") diskFull = true; + if (scratchExhaustionError(error)) diskFull = true; process.stderr.write(`Runtime test process could not start: ${String(error)}\n`); resolve(1); }); - child.once("close", (code) => resolve(code ?? 1)); + child.once("close", (code) => { + childClosed = true; + childExitCode = code; + resolve(code ?? 1); + }); }); if (markerError !== undefined) { process.stderr.write(`Runtime test scratch owner could not be recorded: ${String(markerError)}\n`); status = 1; } if (diskFull) { - process.stderr.write(`Runtime tests exhausted scratch space (ENOSPC) under ${scratch}.\n`); + process.stderr.write(`Runtime tests exhausted scratch space or quota (ENOSPC/EDQUOT) under ${scratch}.\n`); status = 1; } } finally { - try { - removeScratch(scratch); - } catch (error) { - process.stderr.write(`Runtime test scratch cleanup failed for ${scratch}: ${String(error)}\n`); + for (const [signal, handler] of signalHandlers) process.off(signal, handler); + let groupActive = process.platform !== "win32" && childGroupId !== undefined && processGroupIsAlive(childGroupId); + let scratchUse = scratchUseStatus(scratch); + // A worker or unrelated process can exit during /proc enumeration. + // Retry brief ambiguity before retaining an otherwise completed run. + for ( + let attempt = 0; + attempt < 5 && (groupActive || (process.platform === "linux" && scratchUse === "uncertain")); + attempt += 1 + ) { + await new Promise((resolve) => setTimeout(resolve, 50)); + groupActive = process.platform !== "win32" && childGroupId !== undefined && processGroupIsAlive(childGroupId); + scratchUse = scratchUseStatus(scratch); + } + // Non-Linux platforms have no proc scan. A closed POSIX runner with a dead + // group can release its scratch; Windows requires a normal successful exit. + const completedOutsideLinux = + process.platform !== "linux" && + childClosed && + Number.isInteger(childExitCode) && + !groupActive && + (process.platform !== "win32" || childExitCode === 0); + if (groupActive || (scratchUse !== "clear" && !completedOutsideLinux)) { + process.stderr.write( + `Runtime test scratch retained for a possibly active test process (group ${groupActive ? "active" : "dead"}, scratch ${scratchUse}): ${scratch}\n` + ); status = 1; + } else { + try { + removeScratch(scratch); + } catch (error) { + process.stderr.write(`Runtime test scratch cleanup failed for ${scratch}: ${String(error)}\n`); + status = 1; + } } } return status; diff --git a/packages/runtime/test/temporary-root.test.ts b/packages/runtime/test/temporary-root.test.ts index 39c96ce10..6abebdb1f 100644 --- a/packages/runtime/test/temporary-root.test.ts +++ b/packages/runtime/test/temporary-root.test.ts @@ -9,6 +9,8 @@ import { registerTemporaryPath, temporaryRoot } from "./temporary-root.js"; let sealedRoot: string; let adjacentRoot: string; let sharedRoot: string; +let outsideHardlink: string; +let teardownRoot: string; test("test-scoped scratch includes sealed snapshots and registered adjacent paths", () => { sealedRoot = temporaryRoot("ufz-test-cleanup-"); @@ -16,6 +18,11 @@ test("test-scoped scratch includes sealed snapshots and registered adjacent path fs.mkdirSync(snapshot); fs.writeFileSync(path.join(snapshot, "module.js"), "export {};\n"); fs.chmodSync(path.join(snapshot, "module.js"), 0o400); + const outside = temporaryRoot("ufz-hardlink-outside-", "process"); + outsideHardlink = path.join(outside, "shared.js"); + fs.writeFileSync(outsideHardlink, "export {};\n"); + fs.chmodSync(outsideHardlink, 0o400); + fs.linkSync(outsideHardlink, path.join(snapshot, "shared.js")); fs.chmodSync(snapshot, 0o500); adjacentRoot = registerTemporaryPath(fs.mkdtempSync(path.join(path.dirname(sealedRoot), "ufz-test-adjacent-"))); @@ -30,6 +37,17 @@ test("scratch is reclaimed between test cases while a shared fixture remains", ( assert.equal(fs.existsSync(sealedRoot), false); assert.equal(fs.existsSync(adjacentRoot), false); assert.equal(fs.existsSync(sharedRoot), true); + assert.equal(fs.statSync(outsideHardlink).mode & 0o777, 0o400); +}); + +test("scratch remains available to a test's own after hook", (context) => { + teardownRoot = temporaryRoot("ufz-teardown-cleanup-"); + fs.writeFileSync(path.join(teardownRoot, "data"), "fixture"); + context.after(() => assert.equal(fs.readFileSync(path.join(teardownRoot, "data"), "utf8"), "fixture")); +}); + +test("scratch is reclaimed after the test's own after hook", () => { + assert.equal(fs.existsSync(teardownRoot), false); }); test("scratch is reclaimed after a failed test body", () => { diff --git a/packages/runtime/test/temporary-root.ts b/packages/runtime/test/temporary-root.ts index 54d3b24d4..b6980aae9 100644 --- a/packages/runtime/test/temporary-root.ts +++ b/packages/runtime/test/temporary-root.ts @@ -19,8 +19,8 @@ import type { TestContext } from "node:test"; * The root is removable. Sealed execution snapshots are left `dr-x------`, and * unlinking an entry needs the write bit on its parent rather than on the entry * itself, so `rmSync` cannot remove a tree that contains one. Owner write - * permission is restored on files and directories on the way down, which is the same - * treatment `restoreRemovableDirectoryPermissions` applies in `clean.ts`. + * permission is restored on directories on the way down. File modes do not + * affect unlinking, and a fixture file may be hard-linked outside scratch. */ const registered = new Set(); const testRoots = new AsyncLocalStorage(); @@ -29,8 +29,14 @@ let exitHookInstalled = false; /** Attach roots created by a test body to its after hook, including on assertion failure. */ export function withTemporaryRootScope(context: TestContext, body: () => void | Promise): void | Promise { const roots: string[] = []; - context.after(() => removeRoots(roots)); - return testRoots.run(roots, body); + return testRoots.run(roots, async () => { + try { + await body(); + } finally { + // Register after the body so its own after hooks can still use the fixture. + context.after(() => removeRoots(roots)); + } + }); } export function temporaryRoot(prefix: string, lifetime: "test" | "process" = "test"): string { @@ -38,10 +44,12 @@ export function temporaryRoot(prefix: string, lifetime: "test" | "process" = "te try { root = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), prefix)); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOSPC") { - throw new Error(`Runtime test scratch space is full (ENOSPC) while creating ${prefix} in ${os.tmpdir()}`, { - cause: error - }); + const failure = error as NodeJS.ErrnoException; + if (failure.code === "ENOSPC" || failure.code === "EDQUOT" || failure.errno === -122) { + throw new Error( + `Runtime test scratch space or quota is exhausted (ENOSPC/EDQUOT) while creating ${prefix} in ${os.tmpdir()}`, + { cause: error } + ); } throw error; } @@ -115,12 +123,10 @@ function restoreRemovablePermissions(root: string): void { if ((error as NodeJS.ErrnoException).code === "ENOENT") return; throw error; } - if (stat.isSymbolicLink()) return; - if (!stat.isDirectory() && !stat.isFile()) return; - fs.chmodSync(root, stat.mode | (stat.isDirectory() ? 0o700 : 0o600)); if (!stat.isDirectory()) return; + fs.chmodSync(root, stat.mode | 0o700); const entries = fs.readdirSync(root, { withFileTypes: true }); for (const entry of entries) { - if (entry.isDirectory() || entry.isFile()) restoreRemovablePermissions(path.join(root, entry.name)); + if (entry.isDirectory()) restoreRemovablePermissions(path.join(root, entry.name)); } } diff --git a/packages/runtime/test/test-scratch.test.ts b/packages/runtime/test/test-scratch.test.ts new file mode 100644 index 000000000..f2d475743 --- /dev/null +++ b/packages/runtime/test/test-scratch.test.ts @@ -0,0 +1,265 @@ +import assert from "node:assert/strict"; +import { spawn, spawnSync, type ChildProcess } from "node:child_process"; +import { once } from "node:events"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { setTimeout as delay } from "node:timers/promises"; + +const nodeTest = process.versions.bun === undefined ? test : test.skip; +const linuxNodeTest = process.platform === "linux" && process.versions.bun === undefined ? test : test.skip; +const deadLauncherPid = 99_999_999; +const deadChildPid = 99_999_998; + +function runScratchProbe(childCode = ""): ReturnType { + const runner = new URL("../../scripts/test-scratch.mjs", import.meta.url).href; + const script = `import { runTests } from ${JSON.stringify(runner)}; process.exitCode = await runTests(["--eval", ${JSON.stringify(childCode)}]);`; + return spawnSync(process.execPath, ["--input-type=module", "--eval", script], { encoding: "utf8" }); +} + +function liveChildIn(scratch: string): ChildProcess { + return spawn( + process.execPath, + [ + "--eval", + 'const fs = require("node:fs"); fs.writeFileSync(process.env.TMPDIR + "/alive", "yes"); process.stdout.write("ready"); setInterval(() => {}, 1000);' + ], + { env: { ...process.env, TMPDIR: scratch, TMP: scratch, TEMP: scratch }, stdio: ["ignore", "pipe", "pipe"] } + ); +} + +function linuxProcessIsActive(pid: number): boolean { + let stat; + try { + stat = fs.readFileSync(path.join("/proc", String(pid), "stat"), "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; + } + return !/^[ZX]/u.test(stat.slice(stat.lastIndexOf(") ") + 2)); +} + +async function waitForLinuxProcessStop(pid: number): Promise { + for (let attempt = 0; attempt < 100; attempt += 1) { + if (!linuxProcessIsActive(pid)) return true; + await delay(25); + } + return false; +} + +nodeTest("stale scratch is reclaimed only after a complete child marker confirms exit", async (context) => { + const parent = path.join(fs.realpathSync(os.tmpdir()), `ultrafuzz-runtime-tests-${process.getuid?.() ?? "user"}`); + fs.mkdirSync(parent, { mode: 0o700, recursive: true }); + const missing = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); + const torn = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); + fs.writeFileSync(path.join(torn, ".test-child-pgid"), "not-a-pid\n"); + const confirmedDead = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); + fs.writeFileSync(path.join(confirmedDead, ".test-child-pgid"), `${deadChildPid}\n`); + const claimedOriginal = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); + fs.writeFileSync(path.join(claimedOriginal, ".test-child-pgid"), `${deadChildPid}\n`); + for (const directory of [missing, torn, confirmedDead, claimedOriginal]) { + fs.writeFileSync(path.join(directory, ".test-scratch-origin"), `${directory}\n`); + if (process.platform === "linux") { + const stat = fs.readFileSync("/proc/self/stat", "utf8"); + const start = stat + .slice(stat.lastIndexOf(") ") + 2) + .trim() + .split(/\s+/u)[19]; + fs.writeFileSync(path.join(directory, ".test-launcher-start"), `${start}\n`); + } + } + const outside = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), "ufz-scratch-hardlink-")); + const outsideFile = path.join(outside, "shared"); + fs.writeFileSync(outsideFile, "fixture"); + fs.chmodSync(outsideFile, 0o400); + fs.linkSync(outsideFile, path.join(confirmedDead, "shared")); + fs.chmodSync(confirmedDead, 0o500); + + const children = [liveChildIn(missing), liveChildIn(torn), liveChildIn(claimedOriginal)]; + const claimed = path.join(parent, `reap-${deadLauncherPid}-testclaim`); + context.after(async () => { + await Promise.all( + children.map(async (child) => { + if (child.exitCode !== null || child.signalCode !== null) return; + child.kill("SIGTERM"); + await once(child, "exit"); + }) + ); + for (const directory of [missing, torn, confirmedDead, claimedOriginal, claimed, outside]) { + if (fs.existsSync(directory)) fs.chmodSync(directory, 0o700); + fs.rmSync(directory, { recursive: true, force: true }); + } + }); + await Promise.all(children.map(async (child) => once(child.stdout!, "data"))); + fs.renameSync(claimedOriginal, claimed); + + const probe = runScratchProbe(); + assert.equal(probe.status, 0, `${probe.stdout}${probe.stderr}`); + assert.equal(fs.existsSync(path.join(missing, "alive")), true); + assert.equal(fs.existsSync(path.join(torn, "alive")), true); + assert.equal(fs.existsSync(path.join(claimed, "alive")), true); + if (process.platform === "linux") assert.equal(fs.existsSync(confirmedDead), false); + assert.equal(fs.statSync(outsideFile).mode & 0o777, 0o400); + assert.equal( + children.every((child) => child.exitCode === null && child.signalCode === null), + true + ); +}); + +nodeTest("scratch cleanup does not change an outside hard link's mode", (context) => { + const outside = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), "ufz-scratch-hardlink-")); + context.after(() => fs.rmSync(outside, { recursive: true, force: true })); + const outsideFile = path.join(outside, "shared"); + fs.writeFileSync(outsideFile, "fixture"); + fs.chmodSync(outsideFile, 0o400); + const probe = runScratchProbe( + `const fs = require("node:fs"); fs.linkSync(${JSON.stringify(outsideFile)}, process.env.TMPDIR + "/shared"); fs.chmodSync(process.env.TMPDIR, 0o500);` + ); + assert.equal(probe.status, 0, `${probe.stdout}${probe.stderr}`); + assert.equal(fs.statSync(outsideFile).mode & 0o777, 0o400); +}); + +linuxNodeTest("a surviving test worker keeps scratch after its launcher and runner die", async (context) => { + const coordination = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), "ufz-scratch-worker-")); + const ready = path.join(coordination, "ready.json"); + const workerTest = path.join(coordination, "worker.test.mjs"); + fs.writeFileSync( + workerTest, + [ + 'import fs from "node:fs";', + 'import test from "node:test";', + 'test("held worker", async () => {', + ` fs.writeFileSync(${JSON.stringify(ready)}, JSON.stringify({ pid: process.pid, scratch: process.env.TMPDIR }));`, + " await new Promise((resolve) => setTimeout(resolve, 30_000));", + "});" + ].join("\n") + ); + const runner = new URL("../../scripts/test-scratch.mjs", import.meta.url).href; + const launcherCode = `import { runTests } from ${JSON.stringify(runner)}; process.exitCode = await runTests(["--test", ${JSON.stringify(workerTest)}]);`; + const launcherEnv = { ...process.env }; + delete launcherEnv.NODE_TEST_CONTEXT; + const launcher = spawn(process.execPath, ["--input-type=module", "--eval", launcherCode], { + env: launcherEnv, + stdio: ["ignore", "pipe", "pipe"] + }); + let launcherOutput = ""; + launcher.stdout!.on("data", (chunk: Buffer) => (launcherOutput += String(chunk))); + launcher.stderr!.on("data", (chunk: Buffer) => (launcherOutput += String(chunk))); + const live: { runnerPgid?: number; workerPid?: number; scratch?: string } = {}; + context.after(async () => { + if (launcher.exitCode === null && launcher.signalCode === null) { + launcher.kill("SIGKILL"); + await once(launcher, "exit"); + } + if (live.runnerPgid !== undefined) { + try { + process.kill(-live.runnerPgid, "SIGKILL"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } + if (live.workerPid !== undefined) { + try { + process.kill(live.workerPid, "SIGKILL"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } + if (live.workerPid !== undefined && (await waitForLinuxProcessStop(live.workerPid)) && live.scratch !== undefined) { + fs.rmSync(live.scratch, { recursive: true, force: true }); + } + fs.rmSync(coordination, { recursive: true, force: true }); + }); + + for (let attempt = 0; attempt < 200 && !fs.existsSync(ready); attempt += 1) { + assert.equal(launcher.exitCode, null, `test launcher exited before its worker was ready: ${launcherOutput}`); + await delay(25); + } + assert.equal(fs.existsSync(ready), true, "test worker never reported its scratch directory"); + const readyState = JSON.parse(fs.readFileSync(ready, "utf8")) as { pid: number; scratch: string }; + live.workerPid = readyState.pid; + live.scratch = readyState.scratch; + live.runnerPgid = Number(fs.readFileSync(path.join(live.scratch, ".test-child-pgid"), "utf8").trim()); + assert.equal(Number.isSafeInteger(live.runnerPgid), true); + + launcher.kill("SIGKILL"); + await once(launcher, "exit"); + process.kill(live.runnerPgid, "SIGKILL"); + await delay(25); + assert.equal(linuxProcessIsActive(live.workerPid!), true, "test worker stopped before stale reaping began"); + const probe = runScratchProbe(); + assert.equal(probe.status, 0, `${probe.stdout}${probe.stderr}`); + assert.equal(linuxProcessIsActive(live.workerPid!), true, "test worker stopped during stale reaping"); + assert.equal(fs.existsSync(live.scratch), true, "reaper deleted scratch still owned by a live worker"); +}); + +linuxNodeTest("SIGINT to the launcher reaches its detached test worker", async (context) => { + const coordination = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), "ufz-scratch-signal-")); + const ready = path.join(coordination, "ready.json"); + const workerTest = path.join(coordination, "signal.test.mjs"); + fs.writeFileSync( + workerTest, + [ + 'import fs from "node:fs";', + 'import test from "node:test";', + 'test("held worker", async () => {', + ` fs.writeFileSync(${JSON.stringify(ready)}, JSON.stringify({ pid: process.pid, scratch: process.env.TMPDIR }));`, + " await new Promise((resolve) => setTimeout(resolve, 30_000));", + "});" + ].join("\n") + ); + const runner = new URL("../../scripts/test-scratch.mjs", import.meta.url).href; + const launcherCode = `import { runTests } from ${JSON.stringify(runner)}; process.exitCode = await runTests(["--test", ${JSON.stringify(workerTest)}]);`; + const launcherEnv = { ...process.env }; + delete launcherEnv.NODE_TEST_CONTEXT; + const launcher = spawn(process.execPath, ["--input-type=module", "--eval", launcherCode], { + env: launcherEnv, + stdio: ["ignore", "pipe", "pipe"] + }); + const live: { runnerPgid?: number; workerPid?: number; scratch?: string } = {}; + context.after(async () => { + if (launcher.exitCode === null && launcher.signalCode === null) { + launcher.kill("SIGKILL"); + await once(launcher, "exit"); + } + if (live.runnerPgid !== undefined) { + try { + process.kill(-live.runnerPgid, "SIGKILL"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } + if (live.workerPid !== undefined && (await waitForLinuxProcessStop(live.workerPid)) && live.scratch !== undefined) { + fs.rmSync(live.scratch, { recursive: true, force: true }); + } + fs.rmSync(coordination, { recursive: true, force: true }); + }); + + for (let attempt = 0; attempt < 200 && !fs.existsSync(ready); attempt += 1) { + assert.equal(launcher.exitCode, null, "test launcher exited before its worker was ready"); + await delay(25); + } + assert.equal(fs.existsSync(ready), true, "test worker never reported its scratch directory"); + const readyState = JSON.parse(fs.readFileSync(ready, "utf8")) as { pid: number; scratch: string }; + live.workerPid = readyState.pid; + live.scratch = readyState.scratch; + live.runnerPgid = Number(fs.readFileSync(path.join(live.scratch, ".test-child-pgid"), "utf8").trim()); + assert.equal(Number.isSafeInteger(live.runnerPgid), true); + + launcher.kill("SIGINT"); + await Promise.race([ + once(launcher, "exit"), + delay(5_000, undefined, { ref: false }).then(() => assert.fail("launcher ignored SIGINT")) + ]); + assert.notEqual(launcher.exitCode, 0); + assert.equal(await waitForLinuxProcessStop(live.workerPid), true, "test worker remained active after SIGINT"); +}); + +nodeTest("scratch runner identifies quota exhaustion reported by Node or the filesystem", () => { + for (const symptom of ["Error: EDQUOT\n", "Error: Unknown system error -122\n"]) { + const probe = runScratchProbe(`process.stderr.write(${JSON.stringify(symptom)});`); + assert.equal(probe.status, 1, `${probe.stdout}${probe.stderr}`); + assert.match(`${probe.stdout}${probe.stderr}`, /exhausted scratch space or quota \(ENOSPC\/EDQUOT\)/u); + } +}); From c1f09c93080eb73586e2bc02c1ce4159ec695ce9 Mon Sep 17 00:00:00 2001 From: Antonio Viggiano Date: Mon, 26 Oct 2026 20:27:59 +0000 Subject: [PATCH 4/5] fix(runtime): handle scratch recovery races and strict lint --- packages/runtime/scripts/test-scratch.mjs | 468 ++++++++++++--------- packages/runtime/test/test-scratch.test.ts | 102 ++++- 2 files changed, 368 insertions(+), 202 deletions(-) diff --git a/packages/runtime/scripts/test-scratch.mjs b/packages/runtime/scripts/test-scratch.mjs index a14465a95..4d9482c60 100644 --- a/packages/runtime/scripts/test-scratch.mjs +++ b/packages/runtime/scripts/test-scratch.mjs @@ -32,7 +32,23 @@ function pidIsAlive(pid) { function processGroupIsAlive(pgid) { // A Node --test runner can exit while its per-file workers keep running. // On Windows a group probe is unavailable, so stale scratch is retained. - return process.platform === "win32" || pidIsAlive(-pgid); + if (process.platform === "win32") return true; + if (!pidIsAlive(-pgid)) return false; + if (process.platform !== "linux") return true; + let processes; + try { + processes = fs.readdirSync("/proc", { withFileTypes: true }); + } catch { + return true; + } + let uncertain = false; + for (const entry of processes) { + if (!/^[0-9]+$/u.test(entry.name)) continue; + const info = processInfo(entry.name); + if (info?.group === pgid && info.state !== "Z" && info.state !== "X") return true; + if (info === undefined && pidIsAlive(Number(entry.name))) uncertain = true; + } + return uncertain; } function recordedTestGroupIsDead(root) { @@ -58,22 +74,30 @@ function pathIsWithin(candidate, root) { return candidate === root || candidate.startsWith(`${root}${path.sep}`); } -function processStartTicks(pid) { +function processInfo(pid) { try { const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); const commandEnd = stat.lastIndexOf(") "); if (commandEnd === -1) return undefined; // Field 22 is starttime; the text after the command starts at field 3. - const value = stat + const fields = stat .slice(commandEnd + 2) .trim() - .split(/\s+/u)[19]; - return /^[0-9]+$/u.test(value ?? "") ? BigInt(value) : undefined; + .split(/\s+/u); + const value = fields[19]; + const group = Number(fields[2]); // Field 5 is the process group ID. + return /^[0-9]+$/u.test(value ?? "") && Number.isSafeInteger(group) + ? { start: BigInt(value), state: fields[0], group } + : undefined; } catch { return undefined; } } +function processStartTicks(pid) { + return processInfo(pid)?.start; +} + function recordedLauncherStart(root) { let marker; try { @@ -84,11 +108,94 @@ function recordedLauncherStart(root) { return /^[0-9]+\n$/u.test(marker) ? BigInt(marker.trim()) : undefined; } +function sameUidStatus(processRoot) { + try { + // Non-dumpable processes can have root-owned /proc directories. + const status = fs.readFileSync(path.join(processRoot, "status"), "utf8"); + const match = /^Uid:\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)/mu.exec(status); + if (match === null) return "uncertain"; + return match.slice(1).some((uid) => Number(uid) === process.getuid()) ? "same" : "other"; + } catch { + return "uncertain"; + } +} + +function environmentUseStatus(processRoot, paths) { + try { + const environment = fs.readFileSync(path.join(processRoot, "environ"), "utf8"); + const usesScratch = environment + .split("\0") + .some((field) => + paths.some( + (directory) => + field === `TMPDIR=${directory}` || field === `TMP=${directory}` || field === `TEMP=${directory}` + ) + ); + return usesScratch ? "active" : "clear"; + } catch { + return "uncertain"; + } +} + +function cwdUseStatus(processRoot, paths) { + try { + const cwd = fs.readlinkSync(path.join(processRoot, "cwd")); + return paths.some((directory) => pathIsWithin(cwd, directory)) ? "active" : "clear"; + } catch { + return "uncertain"; + } +} + +function fdUseStatus(processRoot, paths) { + let descriptors; + try { + descriptors = fs.readdirSync(path.join(processRoot, "fd")); + } catch { + return "uncertain"; + } + let uncertain = false; + for (const descriptor of descriptors) { + let target; + try { + target = fs.readlinkSync(path.join(processRoot, "fd", descriptor)); + } catch (error) { + if (error.code !== "ENOENT") uncertain = true; + continue; // A descriptor may close during the scan. + } + if (paths.some((directory) => pathIsWithin(target, directory))) return "active"; + } + return uncertain ? "uncertain" : "clear"; +} + +function uncertainProcessIsGoneOrOlder(name, firstInfo, launcherStart) { + const currentInfo = processInfo(name); + if (currentInfo?.state === "Z" || currentInfo?.state === "X") return true; + if (currentInfo === undefined && !pidIsAlive(Number(name))) return true; + // Older processes could receive the path later, so readable cwd/fds were + // checked first. Age only excuses unavailable /proc reads. + return firstInfo?.start < launcherStart && currentInfo?.start < launcherStart; +} + +function processScratchUseStatus(name, paths, launcherStart) { + const processRoot = path.join("/proc", name); + const firstInfo = processInfo(name); + if (firstInfo?.state === "Z" || firstInfo?.state === "X") return "clear"; + const ownership = sameUidStatus(processRoot); + if (ownership === "other") return "clear"; + const environment = environmentUseStatus(processRoot, paths); + if (environment === "active") return "active"; + const cwd = cwdUseStatus(processRoot, paths); + if (cwd === "active") return "active"; + const fds = fdUseStatus(processRoot, paths); + if (fds === "active") return "active"; + if (ownership === "same" && environment === "clear" && cwd === "clear" && fds === "clear") return "clear"; + return uncertainProcessIsGoneOrOlder(name, firstInfo, launcherStart) ? "clear" : "uncertain"; +} + function scratchUseStatus(scratch, original = scratch) { if (process.platform !== "linux" || process.getuid === undefined) return "uncertain"; const launcherStart = recordedLauncherStart(scratch); if (launcherStart === undefined) return "uncertain"; - const paths = [original, scratch]; let processes; try { processes = fs.readdirSync("/proc", { withFileTypes: true }); @@ -98,67 +205,9 @@ function scratchUseStatus(scratch, original = scratch) { let uncertain = false; for (const entry of processes) { if (!/^[0-9]+$/u.test(entry.name)) continue; - const pid = Number(entry.name); - const processRoot = path.join("/proc", entry.name); - const started = processStartTicks(entry.name); - // A process that started before this launcher predates its scratch. - // Readable cwd/fds may still show later use; age excuses only read errors. - const olderProcess = started !== undefined && started < launcherStart; - let readUncertain = false; - try { - // Non-dumpable processes can have root-owned /proc directories even - // while their real/effective UIDs still match this test launcher. - const status = fs.readFileSync(path.join(processRoot, "status"), "utf8"); - const match = /^Uid:\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)/mu.exec(status); - if (match === null) readUncertain = true; - else if (!match.slice(1).some((uid) => Number(uid) === process.getuid())) continue; - } catch (error) { - if ((error.code === "ENOENT" || error.code === "ESRCH") && !pidIsAlive(pid)) continue; - readUncertain = true; - } - let environment; - try { - environment = fs.readFileSync(path.join(processRoot, "environ"), "utf8"); - } catch (error) { - if ((error.code === "ENOENT" || error.code === "ESRCH") && !pidIsAlive(pid)) continue; - readUncertain = true; - } - if ( - environment - ?.split("\0") - .some((field) => - paths.some( - (directory) => - field === `TMPDIR=${directory}` || field === `TMP=${directory}` || field === `TEMP=${directory}` - ) - ) - ) { - return "active"; - } - try { - const cwd = fs.readlinkSync(path.join(processRoot, "cwd")); - if (paths.some((directory) => pathIsWithin(cwd, directory))) return "active"; - for (const descriptor of fs.readdirSync(path.join(processRoot, "fd"))) { - let target; - try { - target = fs.readlinkSync(path.join(processRoot, "fd", descriptor)); - } catch (error) { - if (error.code === "ENOENT" && !pidIsAlive(pid)) break; - if (error.code === "ENOENT") continue; // A descriptor closed during the scan. - throw error; - } - if (paths.some((directory) => pathIsWithin(target, directory))) return "active"; - } - } catch (error) { - if ((error.code === "ENOENT" || error.code === "ESRCH") && !pidIsAlive(pid)) continue; - readUncertain = true; - } - if (readUncertain) { - if (!pidIsAlive(pid)) continue; - const currentStart = processStartTicks(entry.name); - const stillOlder = currentStart !== undefined && currentStart < launcherStart; - if (!olderProcess || !stillOlder) uncertain = true; - } + const use = processScratchUseStatus(entry.name, [original, scratch], launcherStart); + if (use === "active") return "active"; + if (use === "uncertain") uncertain = true; } return uncertain ? "uncertain" : "clear"; } @@ -187,6 +236,49 @@ function recordedOriginalScratch(root, parent) { return original; } +function reapStaleScratch(parent, entry) { + if (!entry.isDirectory() || process.platform !== "linux") return; + const match = /^(?:run|reap)-([1-9][0-9]*)-[A-Za-z0-9]+$/u.exec(entry.name); + if (match === null) return; + const stale = path.join(parent, entry.name); + const launcherStart = recordedLauncherStart(stale); + if (launcherStart === undefined) return; + const currentStart = processStartTicks(match[1]); + if (currentStart === launcherStart || (currentStart === undefined && pidIsAlive(Number(match[1])))) return; + const original = recordedOriginalScratch(stale, parent); + if (original === undefined || !recordedTestGroupIsDead(stale) || scratchUseStatus(stale, original) !== "clear") + return; + const claimed = path.join(parent, `reap-${process.pid}-${randomUUID().replaceAll("-", "")}`); + try { + fs.renameSync(stale, claimed); + } catch (error) { + if (error.code === "ENOENT") return; // Another test launcher claimed it first. + throw error; + } + // An escaped child still has the original run path after the rename. + if (recordedTestGroupIsDead(claimed) && scratchUseStatus(claimed, original) === "clear") removeScratch(claimed); +} + +function createScratch(parent) { + const scratch = fs.mkdtempSync(path.join(parent, `run-${process.pid}-`)); + try { + fs.writeFileSync(path.join(scratch, ".test-scratch-origin"), `${scratch}\n`, { mode: 0o600, flag: "wx" }); + if (process.platform === "linux") { + const start = processStartTicks("self"); + if (start === undefined) throw new Error("Runtime test launcher start time is unavailable from /proc/self/stat"); + fs.writeFileSync(path.join(scratch, ".test-launcher-start"), `${start}\n`, { mode: 0o600, flag: "wx" }); + } + } catch (error) { + try { + removeScratch(scratch); + } catch (cleanupError) { + process.stderr.write(`Runtime test scratch setup cleanup failed for ${scratch}: ${String(cleanupError)}\n`); + } + throw error; + } + return scratch; +} + function ownedScratchDirectory() { const parent = path.join(fs.realpathSync(os.tmpdir()), `ultrafuzz-runtime-tests-${process.getuid?.() ?? "user"}`); fs.mkdirSync(parent, { mode: 0o700, recursive: true }); @@ -198,145 +290,145 @@ function ownedScratchDirectory() { ) { throw new Error(`Runtime test scratch parent is not owned by this process: ${parent}`); } - for (const entry of fs.readdirSync(parent, { withFileTypes: true })) { - const match = /^(?:run|reap)-([1-9][0-9]*)-[A-Za-z0-9]+$/u.exec(entry.name); - if (!match || !entry.isDirectory() || pidIsAlive(Number(match[1]))) continue; - const stale = path.join(parent, entry.name); - const original = recordedOriginalScratch(stale, parent); - if (original === undefined || !recordedTestGroupIsDead(stale) || scratchUseStatus(stale, original) !== "clear") - continue; - const claimed = path.join(parent, `reap-${process.pid}-${randomUUID().replaceAll("-", "")}`); - try { - fs.renameSync(stale, claimed); - } catch (error) { - if (error.code === "ENOENT") continue; // Another test launcher claimed it first. - throw error; - } - // A process can start while we claim the directory, and an escaped child - // still has the original run path in its environment after the rename. - if (recordedTestGroupIsDead(claimed) && scratchUseStatus(claimed, original) === "clear") removeScratch(claimed); - } - const scratch = fs.mkdtempSync(path.join(parent, `run-${process.pid}-`)); - fs.writeFileSync(path.join(scratch, ".test-scratch-origin"), `${scratch}\n`, { mode: 0o600, flag: "wx" }); - if (process.platform === "linux") { - const start = processStartTicks("self"); - if (start === undefined) throw new Error("Runtime test launcher start time is unavailable from /proc/self/stat"); - fs.writeFileSync(path.join(scratch, ".test-launcher-start"), `${start}\n`, { mode: 0o600, flag: "wx" }); - } - return scratch; + for (const entry of fs.readdirSync(parent, { withFileTypes: true })) reapStaleScratch(parent, entry); + return createScratch(parent); } -export async function runTests(args, extraEnv = {}) { - let scratch; +function forwardTestSignals(groupId) { + if (process.platform === "win32" || groupId === undefined) return () => {}; + const handlers = ["SIGINT", "SIGTERM", "SIGHUP"].map((signal) => { + const handler = () => signalTestGroup(groupId, signal); + process.on(signal, handler); + return [signal, handler]; + }); + return () => { + for (const [signal, handler] of handlers) process.off(signal, handler); + }; +} + +function recordTestGroup(scratch, child) { + if (child.pid === undefined) return undefined; try { - scratch = ownedScratchDirectory(); + const pendingMarker = path.join(scratch, ".test-child-pgid.pending"); + fs.writeFileSync(pendingMarker, `${child.pid}\n`, { mode: 0o600, flag: "wx" }); + fs.renameSync(pendingMarker, path.join(scratch, ".test-child-pgid")); + return undefined; } catch (error) { - process.stderr.write( - `Runtime test scratch setup failed${scratchExhaustionError(error) ? " (disk full or quota exceeded, ENOSPC/EDQUOT)" : ""}: ${String(error)}\n` - ); - return 1; + if (process.platform === "win32") child.kill("SIGKILL"); + else signalTestGroup(child.pid, "SIGKILL"); + return error; } +} + +function observeTestOutput(child, observation) { + for (const [stream, output] of [ + [child.stdout, process.stdout], + [child.stderr, process.stderr] + ]) { + let tail = ""; + stream.on("data", (chunk) => { + const value = tail + String(chunk); + if (scratchExhaustionOutput.test(value)) observation.diskFull = true; + tail = value.slice(-40); + }); + stream.pipe(output, { end: false }); + } +} + +function waitForTestChild(child, observation) { + return new Promise((resolve) => { + child.once("error", (error) => { + if (scratchExhaustionError(error)) observation.diskFull = true; + process.stderr.write(`Runtime test process could not start: ${String(error)}\n`); + resolve({ status: 1, closed: false, exitCode: undefined }); + }); + child.once("close", (code) => resolve({ status: code ?? 1, closed: true, exitCode: code })); + }); +} - let status; - let diskFull = false; - let childGroupId; - let childClosed = false; - let childExitCode; - const signalHandlers = []; +async function runTestChild(args, extraEnv, scratch) { + let child; + let stopForwarding = () => {}; + const observation = { diskFull: false }; try { - const child = spawn(process.execPath, args, { + child = spawn(process.execPath, args, { env: { ...process.env, ...extraEnv, TMPDIR: scratch, TMP: scratch, TEMP: scratch }, stdio: ["inherit", "pipe", "pipe"], detached: process.platform !== "win32" }); - childGroupId = child.pid; - if (process.platform !== "win32" && childGroupId !== undefined) { - for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) { - const handler = () => signalTestGroup(childGroupId, signal); - process.on(signal, handler); - signalHandlers.push([signal, handler]); - } - } - let markerError; - if (child.pid !== undefined) { - try { - const pendingMarker = path.join(scratch, ".test-child-pgid.pending"); - fs.writeFileSync(pendingMarker, `${child.pid}\n`, { mode: 0o600, flag: "wx" }); - fs.renameSync(pendingMarker, path.join(scratch, ".test-child-pgid")); - } catch (error) { - markerError = error; - if (process.platform === "win32") child.kill("SIGKILL"); - else signalTestGroup(child.pid, "SIGKILL"); - } - } - for (const [stream, output] of [ - [child.stdout, process.stdout], - [child.stderr, process.stderr] - ]) { - let tail = ""; - stream.on("data", (chunk) => { - const text = tail + String(chunk); - if (scratchExhaustionOutput.test(text)) diskFull = true; - tail = text.slice(-40); - }); - stream.pipe(output, { end: false }); - } - status = await new Promise((resolve) => { - child.once("error", (error) => { - if (scratchExhaustionError(error)) diskFull = true; - process.stderr.write(`Runtime test process could not start: ${String(error)}\n`); - resolve(1); - }); - child.once("close", (code) => { - childClosed = true; - childExitCode = code; - resolve(code ?? 1); - }); - }); + stopForwarding = forwardTestSignals(child.pid); + const markerError = recordTestGroup(scratch, child); + observeTestOutput(child, observation); + const result = await waitForTestChild(child, observation); + let status = result.status; if (markerError !== undefined) { process.stderr.write(`Runtime test scratch owner could not be recorded: ${String(markerError)}\n`); status = 1; } - if (diskFull) { + if (observation.diskFull) { process.stderr.write(`Runtime tests exhausted scratch space or quota (ENOSPC/EDQUOT) under ${scratch}.\n`); status = 1; } + return { ...result, status, groupId: child.pid }; + } catch (error) { + process.stderr.write(`Runtime test process failed: ${String(error)}\n`); + return { status: 1, closed: false, exitCode: undefined, groupId: child?.pid }; } finally { - for (const [signal, handler] of signalHandlers) process.off(signal, handler); - let groupActive = process.platform !== "win32" && childGroupId !== undefined && processGroupIsAlive(childGroupId); - let scratchUse = scratchUseStatus(scratch); - // A worker or unrelated process can exit during /proc enumeration. - // Retry brief ambiguity before retaining an otherwise completed run. - for ( - let attempt = 0; - attempt < 5 && (groupActive || (process.platform === "linux" && scratchUse === "uncertain")); - attempt += 1 - ) { - await new Promise((resolve) => setTimeout(resolve, 50)); - groupActive = process.platform !== "win32" && childGroupId !== undefined && processGroupIsAlive(childGroupId); - scratchUse = scratchUseStatus(scratch); - } - // Non-Linux platforms have no proc scan. A closed POSIX runner with a dead - // group can release its scratch; Windows requires a normal successful exit. - const completedOutsideLinux = - process.platform !== "linux" && - childClosed && - Number.isInteger(childExitCode) && - !groupActive && - (process.platform !== "win32" || childExitCode === 0); - if (groupActive || (scratchUse !== "clear" && !completedOutsideLinux)) { - process.stderr.write( - `Runtime test scratch retained for a possibly active test process (group ${groupActive ? "active" : "dead"}, scratch ${scratchUse}): ${scratch}\n` - ); - status = 1; - } else { - try { - removeScratch(scratch); - } catch (error) { - process.stderr.write(`Runtime test scratch cleanup failed for ${scratch}: ${String(error)}\n`); - status = 1; - } - } + stopForwarding(); + } +} + +function currentTestGroupIsActive(groupId) { + return process.platform !== "win32" && groupId !== undefined && processGroupIsAlive(groupId); +} + +async function cleanupCurrentScratch(scratch, result) { + let groupActive = currentTestGroupIsActive(result.groupId); + let scratchUse = scratchUseStatus(scratch); + // A worker or unrelated process can exit during /proc enumeration. + for ( + let attempt = 0; + attempt < 5 && (groupActive || (process.platform === "linux" && scratchUse === "uncertain")); + attempt += 1 + ) { + await new Promise((resolve) => setTimeout(resolve, 50)); + groupActive = currentTestGroupIsActive(result.groupId); + scratchUse = scratchUseStatus(scratch); + } + // Non-Linux platforms have no proc scan. A closed POSIX runner with a dead + // group can release scratch; Windows requires a normal successful exit. + const completedOutsideLinux = + process.platform !== "linux" && + result.closed && + Number.isInteger(result.exitCode) && + !groupActive && + (process.platform !== "win32" || result.exitCode === 0); + if (groupActive || (scratchUse !== "clear" && !completedOutsideLinux)) { + process.stderr.write( + `Runtime test scratch retained for a possibly active test process (group ${groupActive ? "active" : "dead"}, scratch ${scratchUse}): ${scratch}\n` + ); + // Unrelated processes can make /proc uncertain; preserve test status. + return groupActive || scratchUse === "active" ? 1 : result.status; + } + try { + removeScratch(scratch); + } catch (error) { + process.stderr.write(`Runtime test scratch cleanup failed for ${scratch}: ${String(error)}\n`); + return 1; + } + return result.status; +} + +export async function runTests(args, extraEnv = {}) { + let scratch; + try { + scratch = ownedScratchDirectory(); + } catch (error) { + process.stderr.write( + `Runtime test scratch setup failed${scratchExhaustionError(error) ? " (disk full or quota exceeded, ENOSPC/EDQUOT)" : ""}: ${String(error)}\n` + ); + return 1; } - return status; + const result = await runTestChild(args, extraEnv, scratch); + return cleanupCurrentScratch(scratch, result); } diff --git a/packages/runtime/test/test-scratch.test.ts b/packages/runtime/test/test-scratch.test.ts index f2d475743..84f2a3253 100644 --- a/packages/runtime/test/test-scratch.test.ts +++ b/packages/runtime/test/test-scratch.test.ts @@ -56,9 +56,11 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms fs.writeFileSync(path.join(torn, ".test-child-pgid"), "not-a-pid\n"); const confirmedDead = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); fs.writeFileSync(path.join(confirmedDead, ".test-child-pgid"), `${deadChildPid}\n`); + const reusedPid = fs.mkdtempSync(path.join(parent, `run-${process.pid}-`)); + fs.writeFileSync(path.join(reusedPid, ".test-child-pgid"), `${deadChildPid}\n`); const claimedOriginal = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); fs.writeFileSync(path.join(claimedOriginal, ".test-child-pgid"), `${deadChildPid}\n`); - for (const directory of [missing, torn, confirmedDead, claimedOriginal]) { + for (const directory of [missing, torn, confirmedDead, reusedPid, claimedOriginal]) { fs.writeFileSync(path.join(directory, ".test-scratch-origin"), `${directory}\n`); if (process.platform === "linux") { const stat = fs.readFileSync("/proc/self/stat", "utf8"); @@ -66,7 +68,9 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms .slice(stat.lastIndexOf(") ") + 2) .trim() .split(/\s+/u)[19]; - fs.writeFileSync(path.join(directory, ".test-launcher-start"), `${start}\n`); + if (start === undefined) throw new Error("test process start time is unavailable"); + const marker = directory === reusedPid ? BigInt(start) - 1n : BigInt(start); + fs.writeFileSync(path.join(directory, ".test-launcher-start"), `${marker}\n`); } } const outside = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), "ufz-scratch-hardlink-")); @@ -86,20 +90,32 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms await once(child, "exit"); }) ); - for (const directory of [missing, torn, confirmedDead, claimedOriginal, claimed, outside]) { + for (const directory of [missing, torn, confirmedDead, reusedPid, claimedOriginal, claimed, outside]) { if (fs.existsSync(directory)) fs.chmodSync(directory, 0o700); fs.rmSync(directory, { recursive: true, force: true }); } }); - await Promise.all(children.map(async (child) => once(child.stdout!, "data"))); + await Promise.all( + children.map(async (child) => { + assert.ok(child.stdout); + return once(child.stdout, "data"); + }) + ); fs.renameSync(claimedOriginal, claimed); - const probe = runScratchProbe(); - assert.equal(probe.status, 0, `${probe.stdout}${probe.stderr}`); - assert.equal(fs.existsSync(path.join(missing, "alive")), true); - assert.equal(fs.existsSync(path.join(torn, "alive")), true); - assert.equal(fs.existsSync(path.join(claimed, "alive")), true); - if (process.platform === "linux") assert.equal(fs.existsSync(confirmedDead), false); + for (let attempt = 0; attempt < 10; attempt += 1) { + const probe = runScratchProbe(); + if (probe.status !== 0) assert.match(String(probe.stderr), /scratch retained for a possibly active test process/u); + assert.equal(fs.existsSync(path.join(missing, "alive")), true); + assert.equal(fs.existsSync(path.join(torn, "alive")), true); + assert.equal(fs.existsSync(path.join(claimed, "alive")), true); + if (process.platform !== "linux" || (!fs.existsSync(confirmedDead) && !fs.existsSync(reusedPid))) break; + await delay(50); + } + if (process.platform === "linux") { + assert.equal(fs.existsSync(confirmedDead), false); + assert.equal(fs.existsSync(reusedPid), false, "reaper treated a reused PID as the original launcher"); + } assert.equal(fs.statSync(outsideFile).mode & 0o777, 0o400); assert.equal( children.every((child) => child.exitCode === null && child.signalCode === null), @@ -144,8 +160,10 @@ linuxNodeTest("a surviving test worker keeps scratch after its launcher and runn stdio: ["ignore", "pipe", "pipe"] }); let launcherOutput = ""; - launcher.stdout!.on("data", (chunk: Buffer) => (launcherOutput += String(chunk))); - launcher.stderr!.on("data", (chunk: Buffer) => (launcherOutput += String(chunk))); + assert.ok(launcher.stdout); + assert.ok(launcher.stderr); + launcher.stdout.on("data", (chunk: Buffer) => (launcherOutput += String(chunk))); + launcher.stderr.on("data", (chunk: Buffer) => (launcherOutput += String(chunk))); const live: { runnerPgid?: number; workerPid?: number; scratch?: string } = {}; context.after(async () => { if (launcher.exitCode === null && launcher.signalCode === null) { @@ -178,6 +196,7 @@ linuxNodeTest("a surviving test worker keeps scratch after its launcher and runn } assert.equal(fs.existsSync(ready), true, "test worker never reported its scratch directory"); const readyState = JSON.parse(fs.readFileSync(ready, "utf8")) as { pid: number; scratch: string }; + const workerPid = readyState.pid; live.workerPid = readyState.pid; live.scratch = readyState.scratch; live.runnerPgid = Number(fs.readFileSync(path.join(live.scratch, ".test-child-pgid"), "utf8").trim()); @@ -187,10 +206,10 @@ linuxNodeTest("a surviving test worker keeps scratch after its launcher and runn await once(launcher, "exit"); process.kill(live.runnerPgid, "SIGKILL"); await delay(25); - assert.equal(linuxProcessIsActive(live.workerPid!), true, "test worker stopped before stale reaping began"); + assert.equal(linuxProcessIsActive(workerPid), true, "test worker stopped before stale reaping began"); const probe = runScratchProbe(); assert.equal(probe.status, 0, `${probe.stdout}${probe.stderr}`); - assert.equal(linuxProcessIsActive(live.workerPid!), true, "test worker stopped during stale reaping"); + assert.equal(linuxProcessIsActive(workerPid), true, "test worker stopped during stale reaping"); assert.equal(fs.existsSync(live.scratch), true, "reaper deleted scratch still owned by a live worker"); }); @@ -263,3 +282,58 @@ nodeTest("scratch runner identifies quota exhaustion reported by Node or the fil assert.match(`${probe.stdout}${probe.stderr}`, /exhausted scratch space or quota \(ENOSPC\/EDQUOT\)/u); } }); + +nodeTest("a failed scratch marker write removes the new markerless directory", () => { + const runner = new URL("../../scripts/test-scratch.mjs", import.meta.url).href; + const markers = + process.platform === "linux" ? [".test-scratch-origin", ".test-launcher-start"] : [".test-scratch-origin"]; + for (const marker of markers) { + const script = ` + import fs from "node:fs"; + import os from "node:os"; + import path from "node:path"; + import { runTests } from ${JSON.stringify(runner)}; + const originalWrite = fs.writeFileSync; + fs.writeFileSync = (target, ...args) => { + if (String(target).endsWith(${JSON.stringify(marker)})) { + throw Object.assign(new Error("quota exceeded"), { code: "EDQUOT", errno: -122 }); + } + return originalWrite(target, ...args); + }; + const status = await runTests(["--eval", ""]); + const parent = path.join(fs.realpathSync(os.tmpdir()), "ultrafuzz-runtime-tests-" + (process.getuid?.() ?? "user")); + const leaked = fs.readdirSync(parent).some((name) => name.startsWith("run-" + process.pid + "-")); + if (leaked) process.stderr.write("markerless scratch leaked\\n"); + process.exitCode = leaked ? 2 : status; + `; + const probe = spawnSync(process.execPath, ["--input-type=module", "--eval", script], { encoding: "utf8" }); + assert.equal(probe.status, 1, `${marker}: ${probe.stdout}${probe.stderr}`); + assert.match(probe.stderr, /disk full or quota exceeded, ENOSPC\/EDQUOT/u); + } +}); + +linuxNodeTest("an uncertain proc scan retains scratch without failing a passing run", (context) => { + const runner = new URL("../../scripts/test-scratch.mjs", import.meta.url).href; + const script = ` + import fs from "node:fs"; + import { runTests } from ${JSON.stringify(runner)}; + const originalRead = fs.readFileSync; + fs.readFileSync = (target, ...args) => { + if (String(target) === "/proc/" + process.pid + "/environ") { + throw Object.assign(new Error("proc read denied"), { code: "EACCES" }); + } + return originalRead(target, ...args); + }; + process.exitCode = await runTests(["--eval", ""]); + `; + const probe = spawnSync(process.execPath, ["--input-type=module", "--eval", script], { encoding: "utf8" }); + const retained = /scratch retained for a possibly active test process \(group dead, scratch uncertain\): (.+)/u.exec( + String(probe.stderr) + ); + assert.equal(probe.status, 0, `${probe.stdout}${probe.stderr}`); + assert.ok(retained, "the uncertain scan did not retain scratch"); + const scratch = retained[1]; + assert.ok(scratch); + context.after(() => fs.rmSync(scratch, { recursive: true, force: true })); + assert.equal(fs.existsSync(scratch), true); +}); From 45c63fe7ca27121b2ae558cd13a2363503aa9f1f Mon Sep 17 00:00:00 2001 From: Antonio Viggiano Date: Mon, 26 Oct 2026 20:32:19 +0000 Subject: [PATCH 5/5] fix(runtime): preserve live scratch reaper claims --- packages/runtime/scripts/test-scratch.mjs | 24 +++++++++---- packages/runtime/test/test-scratch.test.ts | 39 ++++++++++++++++------ 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/packages/runtime/scripts/test-scratch.mjs b/packages/runtime/scripts/test-scratch.mjs index 4d9482c60..118ab3f26 100644 --- a/packages/runtime/scripts/test-scratch.mjs +++ b/packages/runtime/scripts/test-scratch.mjs @@ -171,8 +171,9 @@ function uncertainProcessIsGoneOrOlder(name, firstInfo, launcherStart) { const currentInfo = processInfo(name); if (currentInfo?.state === "Z" || currentInfo?.state === "X") return true; if (currentInfo === undefined && !pidIsAlive(Number(name))) return true; - // Older processes could receive the path later, so readable cwd/fds were - // checked first. Age only excuses unavailable /proc reads. + // The harness passes private scratch paths only to processes it launches. + // Check readable older env/cwd/fds first; age excuses only unreadable /proc + // evidence from preexisting host daemons, never newer test descendants. return firstInfo?.start < launcherStart && currentInfo?.start < launcherStart; } @@ -236,19 +237,30 @@ function recordedOriginalScratch(root, parent) { return original; } +function scratchOwnerIsActive(kind, pid, suffix, launcherStart) { + const claimMatch = /^([0-9]+)x[0-9a-f]{32}$/u.exec(suffix); + const ownerStart = kind === "run" ? launcherStart : claimMatch === null ? undefined : BigInt(claimMatch[1]); + const currentStart = processStartTicks(pid); + if (ownerStart !== undefined && currentStart === ownerStart) return true; + // Legacy claims have no recorded start tick. Retain while their owner PID + // exists, even if it has been reused; new claims include the identity. + return (currentStart === undefined || ownerStart === undefined) && pidIsAlive(Number(pid)); +} + function reapStaleScratch(parent, entry) { if (!entry.isDirectory() || process.platform !== "linux") return; - const match = /^(?:run|reap)-([1-9][0-9]*)-[A-Za-z0-9]+$/u.exec(entry.name); + const match = /^(run|reap)-([1-9][0-9]*)-([A-Za-z0-9]+)$/u.exec(entry.name); if (match === null) return; const stale = path.join(parent, entry.name); const launcherStart = recordedLauncherStart(stale); if (launcherStart === undefined) return; - const currentStart = processStartTicks(match[1]); - if (currentStart === launcherStart || (currentStart === undefined && pidIsAlive(Number(match[1])))) return; + if (scratchOwnerIsActive(match[1], match[2], match[3], launcherStart)) return; const original = recordedOriginalScratch(stale, parent); if (original === undefined || !recordedTestGroupIsDead(stale) || scratchUseStatus(stale, original) !== "clear") return; - const claimed = path.join(parent, `reap-${process.pid}-${randomUUID().replaceAll("-", "")}`); + const claimStart = processStartTicks("self"); + if (claimStart === undefined) return; + const claimed = path.join(parent, `reap-${process.pid}-${claimStart}x${randomUUID().replaceAll("-", "")}`); try { fs.renameSync(stale, claimed); } catch (error) { diff --git a/packages/runtime/test/test-scratch.test.ts b/packages/runtime/test/test-scratch.test.ts index 84f2a3253..e9573a7d3 100644 --- a/packages/runtime/test/test-scratch.test.ts +++ b/packages/runtime/test/test-scratch.test.ts @@ -60,16 +60,22 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms fs.writeFileSync(path.join(reusedPid, ".test-child-pgid"), `${deadChildPid}\n`); const claimedOriginal = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); fs.writeFileSync(path.join(claimedOriginal, ".test-child-pgid"), `${deadChildPid}\n`); - for (const directory of [missing, torn, confirmedDead, reusedPid, claimedOriginal]) { + const claimedByLiveOriginal = fs.mkdtempSync(path.join(parent, `run-${deadLauncherPid}-`)); + fs.writeFileSync(path.join(claimedByLiveOriginal, ".test-child-pgid"), `${deadChildPid}\n`); + let ownStart: bigint | undefined; + if (process.platform === "linux") { + const stat = fs.readFileSync("/proc/self/stat", "utf8"); + const start = stat + .slice(stat.lastIndexOf(") ") + 2) + .trim() + .split(/\s+/u)[19]; + if (start === undefined) throw new Error("test process start time is unavailable"); + ownStart = BigInt(start); + } + for (const directory of [missing, torn, confirmedDead, reusedPid, claimedOriginal, claimedByLiveOriginal]) { fs.writeFileSync(path.join(directory, ".test-scratch-origin"), `${directory}\n`); - if (process.platform === "linux") { - const stat = fs.readFileSync("/proc/self/stat", "utf8"); - const start = stat - .slice(stat.lastIndexOf(") ") + 2) - .trim() - .split(/\s+/u)[19]; - if (start === undefined) throw new Error("test process start time is unavailable"); - const marker = directory === reusedPid ? BigInt(start) - 1n : BigInt(start); + if (ownStart !== undefined) { + const marker = directory === reusedPid || directory === claimedByLiveOriginal ? ownStart - 1n : ownStart; fs.writeFileSync(path.join(directory, ".test-launcher-start"), `${marker}\n`); } } @@ -82,6 +88,7 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms const children = [liveChildIn(missing), liveChildIn(torn), liveChildIn(claimedOriginal)]; const claimed = path.join(parent, `reap-${deadLauncherPid}-testclaim`); + const claimedByLive = path.join(parent, `reap-${process.pid}-${ownStart ?? "unknown"}x${"a".repeat(32)}`); context.after(async () => { await Promise.all( children.map(async (child) => { @@ -90,7 +97,17 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms await once(child, "exit"); }) ); - for (const directory of [missing, torn, confirmedDead, reusedPid, claimedOriginal, claimed, outside]) { + for (const directory of [ + missing, + torn, + confirmedDead, + reusedPid, + claimedOriginal, + claimed, + claimedByLiveOriginal, + claimedByLive, + outside + ]) { if (fs.existsSync(directory)) fs.chmodSync(directory, 0o700); fs.rmSync(directory, { recursive: true, force: true }); } @@ -102,6 +119,7 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms }) ); fs.renameSync(claimedOriginal, claimed); + fs.renameSync(claimedByLiveOriginal, claimedByLive); for (let attempt = 0; attempt < 10; attempt += 1) { const probe = runScratchProbe(); @@ -109,6 +127,7 @@ nodeTest("stale scratch is reclaimed only after a complete child marker confirms assert.equal(fs.existsSync(path.join(missing, "alive")), true); assert.equal(fs.existsSync(path.join(torn, "alive")), true); assert.equal(fs.existsSync(path.join(claimed, "alive")), true); + assert.equal(fs.existsSync(claimedByLive), true, "reaper claimed scratch owned by a live reaper"); if (process.platform !== "linux" || (!fs.existsSync(confirmedDead) && !fs.existsSync(reusedPid))) break; await delay(50); }