From 9b557ac3e77c702cb43b4836145233307c7559ce Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 00:53:55 -0800 Subject: [PATCH 1/3] chore(quest): claim quest/m0/gate-test-races.md Quest-Claim: dba50197c046101e From 455b630ef133406724c5fe502859a05fc6c4a779 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 02:49:26 -0800 Subject: [PATCH 2/3] fix(gate): signal a scope from the gate, not a helper the enclosing scope kills A gate inside a Quest scope forwarded its owner's signal with systemctl kill, started in the enclosing cgroup while that scope's owner was still signalling it, so the helper could die of the signal it forwarded. Signal the cgroup tree from the gate itself, repeating until no new process appears. Co-Authored-By: Claude Opus 5.5 --- quest/m0/gate-test-races.md | 40 ---------------------- quest/m0/macos-gate.md | 1 - quest/m0/test-cpu-timeout.md | 4 --- src/cli/supervise.ts | 66 +++++++++++++++++++----------------- 4 files changed, 35 insertions(+), 76 deletions(-) delete mode 100644 quest/m0/gate-test-races.md 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/macos-gate.md b/quest/m0/macos-gate.md index e18c9481..88d4a752 100644 --- a/quest/m0/macos-gate.md +++ b/quest/m0/macos-gate.md @@ -60,5 +60,4 @@ first as runner flakiness). ## Related -- [Gate tests pass deterministically](/quest/m0/gate-test-races.md) - Linux assertion races in the same file - [Test CPU timeout](/quest/m0/test-cpu-timeout.md) - timeouts from load rather than a hang 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..454b9c64 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,7 @@ 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; - process.stderr.write( - `quest: cannot signal every process in ${describe.name}, only its command: ${error}\n`, - ); - child.kill(name); - drain.abort(); - }); + signalGroup(group!, name); }; const terminate = () => { stop(); From 959265674e98e4801d0ee7eafdcc96c6640c7182 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 02:54:14 -0800 Subject: [PATCH 3/3] fix(gate): keep the command-only fallback when the scope cannot be signalled Co-Authored-By: Claude Opus 5.5 --- src/cli/supervise.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/cli/supervise.ts b/src/cli/supervise.ts index 454b9c64..feac04a2 100644 --- a/src/cli/supervise.ts +++ b/src/cli/supervise.ts @@ -378,7 +378,16 @@ function contain( if (!forwards(cancel.signal)) return; const name = requested(cancel.signal); signalled = true; - signalGroup(group!, name); + 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();