diff --git a/quest/m0/gate-test-races.md b/quest/m0/gate-test-races.md deleted file mode 100644 index ef12156c..00000000 --- a/quest/m0/gate-test-races.md +++ /dev/null @@ -1,40 +0,0 @@ -# [S] Gate tests pass deterministically - -## Goal - -`test/gate.test.ts` passes on every run, loaded host or not, with each -intermittent failure below reproduced and fixed at its cause in the test or -in `quest gate`. - -## Plan - -Failures seen (2026-11-10, /quest-complete): - -- `concurrent processes never admit past the headroom, and admit every - caller`: `expected [ Array(12) ] to deeply equal [ Array(12) ]` in - merge-group run - [42676200917](https://github.com/moq-dev/quest/actions/runs/42676200917). -- `lets a phase finish its cleanup once the enclosing scope signals it`: - failed once in a local `quest gate just test` while iterating - [moq-dev/quest#596](https://github.com/moq-dev/quest/pull/596), then passed - three times alone and in a full rerun. - -These are assertion failures, not timeouts, so the likely cause is a race in -the test's setup or in admission and scope signalling, not load alone. Gate -admission and scopes changed in -[moq-dev/quest#602](https://github.com/moq-dev/quest/pull/602) and -[moq-dev/quest#585](https://github.com/moq-dev/quest/pull/585), and -[moq-dev/quest#607](https://github.com/moq-dev/quest/pull/607) is changing -them again: start from whichever has landed, and find the PR that caused -each race by `git blame` once it is reproduced. - -Decided 2026-11-10 (user, /quest-plan follow-up from /quest-complete): - -- Scope: ✅ its own quest, separate from the timeout work (rejected: fold - into [test CPU timeout](/quest/m0/test-cpu-timeout.md); leave it to the - flaky-test watcher). -- Milestone: ✅ m0 (rejected: m1). - -## Related - -- [Test CPU timeout](/quest/m0/test-cpu-timeout.md) - the queue build's timeout failures diff --git a/quest/m0/test-cpu-timeout.md b/quest/m0/test-cpu-timeout.md index 0153f8da..56d77355 100644 --- a/quest/m0/test-cpu-timeout.md +++ b/quest/m0/test-cpu-timeout.md @@ -52,7 +52,3 @@ Open for the implementer, with recommendations: ([gate measured CPU](https://github.com/moq-dev/quest/pull/607) is in flight) over a second implementation. - Remove the per-test wall overrides that only existed to survive load. - -## Related - -- [Gate test races](/quest/m0/gate-test-races.md) - the other queue-build failures, which are assertion races, not timeouts diff --git a/src/cli/supervise.ts b/src/cli/supervise.ts index ff8ed50f..feac04a2 100644 --- a/src/cli/supervise.ts +++ b/src/cli/supervise.ts @@ -2,8 +2,8 @@ // systemd user scope, named once and never reused, so its cgroup holds every descendant that does // not ask another service manager to start it. Other hosts fail explicitly; only the gate settles // for a process group there (see `group` in gate.ts). -import { type ChildProcessWithoutNullStreams, execFile, execFileSync, spawn } from "node:child_process"; -import { closeSync, existsSync, constants as fs, openSync, readFileSync, watch, writeSync } from "node:fs"; +import { type ChildProcessWithoutNullStreams, execFileSync, spawn } from "node:child_process"; +import { closeSync, existsSync, constants as fs, openSync, readdirSync, readFileSync, watch, writeSync } from "node:fs"; import { constants, hostname } from "node:os"; // Cgroup paths are POSIX paths, also in the tests that parse them on Windows. import { join } from "node:path/posix"; @@ -184,26 +184,41 @@ export function kill(group: string): void { } } -/** Whether the cgroup still holds a process, or `otherwise` when that cannot be read. */ -function populatedOr(group: string, otherwise: boolean): boolean { - try { - return populated(group); - } catch { - return otherwise; +/** + * Send `signal` to every process in the cgroup tree, as systemd's default `KillMode=control-group` + * stops a unit; cgroup v2 offers only SIGKILL (`cgroup.kill`) itself. It signals from this process, + * which handles the signal, rather than through a helper such as `systemctl kill`: inside a Quest + * scope, a helper starts in the enclosing cgroup while that scope's owner may still be signalling + * it, and would die of the very signal it forwards. Like systemd, it repeats until a pass finds no + * process it has not signalled, so one forked meanwhile gets it too. + */ +function signalGroup(group: string, signal: NodeJS.Signals): void { + const signalled = new Set(); + for (let fresh = true; fresh;) { + fresh = false; + for (const pid of members(group)) { + if (signalled.has(pid)) continue; + signalled.add(pid); + fresh = true; + try { + process.kill(pid, signal); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } } } -/** - * Send `signal` to every process in the scope, as systemd's default `KillMode=control-group` stops - * a unit, then call `done`. cgroup v2 offers only SIGKILL (`cgroup.kill`) itself. - */ -function signalScope(unit: string, signal: NodeJS.Signals, done: (error?: string) => void): void { - execFile( - "systemctl", - ["--user", "kill", "--kill-whom=all", `--signal=${signal}`, unit], - { encoding: "utf8" }, - (error, _stdout, stderr) => done(error ? stderr.trim() || error.message : undefined), - ); +/** The processes in the cgroup and every cgroup below it; none once it is removed. */ +function members(group: string): number[] { + try { + const own = readFileSync(join(group, "cgroup.procs"), "utf8").split("\n").filter(Boolean).map(Number); + const children = readdirSync(group, { withFileTypes: true }).filter((entry) => entry.isDirectory()); + return [...own, ...children.flatMap((child) => members(join(group, child.name)))]; + } catch (error) { + if (["ENOENT", "ENODEV"].includes((error as NodeJS.ErrnoException).code ?? "")) return []; + throw error; + } } /** The scope's cgroup directory, or undefined once systemd no longer knows the unit. */ @@ -296,7 +311,6 @@ export const systemdScopes: Scopes = { name: unit, unsupported: UNSUPPORTED, launcher: "systemd-run", - signal: (name, done) => signalScope(unit, name, done), emptied: () => stopBound(unit), bound: Boolean(enclosing), }, @@ -315,8 +329,6 @@ interface Containment { unsupported: string; /** The program that starts the handshake shell. */ launcher: string; - /** Send `name` to every process in the cgroup, then call `done`, with why when it could not. */ - signal(name: NodeJS.Signals, done: (error?: string) => void): void; /** Stop what the cgroup's end leaves to systemd, once the cgroup is empty. */ emptied(): Promise; /** @@ -366,15 +378,16 @@ function contain( if (!forwards(cancel.signal)) return; const name = requested(cancel.signal); signalled = true; - describe.signal(name, (error) => { - // The cgroup empties once its last process exits, which leaves nothing to signal. - if (!error || !group || !populatedOr(group, true)) return; + try { + signalGroup(group!, name); + } catch (error) { + // Such as EPERM from a process running as another user: signal what this process can. process.stderr.write( `quest: cannot signal every process in ${describe.name}, only its command: ${error}\n`, ); child.kill(name); drain.abort(); - }); + } }; const terminate = () => { stop();