Skip to content

test(spec): order the sdui cleanup fd count on the child's own post-exec report #18530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-litant merged 1 commit into from
Sep 17, 2026
Merged

test(spec): order the sdui cleanup fd count on the child's own post-exec report #18530

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 98 additions & 18 deletions packages/spec/scripts/gen-sdui-manifest-cleanup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,23 @@
// into "this container is closed for business". This half matters even
// when the kill works, because it is what makes a MISSED kill survivable.
//
// WHAT THIS COUNT MUST BE ORDERED AFTER, AND WHY THE PIDFILE IS NOT IT.
// `sdui_spawn_detached` returns as soon as the pidfile is non-empty, and
// its runner writes that pid on its FIRST line -- before the loop that
// closes the inherited descriptors. Measured on this tree: the pid is
// published 151-297us before the close completes, and the ONLY thing that
// kept a pidfile-ordered reading green was the caller's unrelated 50ms
// polling sleep (margin 10-54ms, from a 0.2ms critical section). So the
// pidfile is a barrier for "the leader exists", never for "the leader has
// let go of my descriptors", and a count ordered on it reads a state the
// spawn legitimately passes through -- observed once in CI as LOCKFDS=1
// with LEADER_ALIVE_AT_COUNT=yes and the leader still `bash`, reproduced
// here by lagging the runner between those two steps. The barrier below is
// the spawned command's OWN post-exec report instead: `exec` replaces the
// runner that does the closing, so that line cannot exist until the close
// has run. We wait for the REPORT, never for a value -- the stub prints
// whatever it counts, so a real leak still arrives as a red.
//
// 3. After the trap, nothing from that session survives — specifically
// including a process that has been reparented to init. That is the case
// `kill "$!"` provably cannot reach, and the test asserts the orphan
Expand Down Expand Up @@ -68,15 +85,33 @@ describe.skipIf(!RUNNABLE)('gen-sdui-manifest.sh cleanup contract', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sdui-cleanup-'));
const lock = path.join(dir, 'caller.lock');
const harness = path.join(dir, 'harness.sh');
const devlog = path.join(dir, 'dev.log');
const release = path.join(dir, 'release');
fs.writeFileSync(lock, '');

// A stub server that reproduces the shape that defeats `kill "$!"`: it
// spawns a helper and then exits, leaving the helper reparented to init
// while still inside the session the script created.
//
// It also takes the in-child reading of property (2) and publishes the two
// facts the harness needs to order itself: this code runs only after `exec`
// replaced the shell that closes the inherited descriptors, so every line
// it prints is causally after that close. It exits on a release file rather
// than a wall-clock timer, so "was the leader alive when we counted?" stops
// being a race against node's boot time; the 60s cap is a self-destruct for
// the case where the harness dies without reaping, never a wait anyone
// relies on.
const stub = [
'const fs = require("node:fs");',
'const { spawn } = require("node:child_process");',
'spawn(process.execPath, ["-e", "setInterval(() => {}, 1e9)"], { stdio: "ignore" });',
'setTimeout(() => process.exit(0), 1200);',
'const LOCK = fs.realpathSync(process.argv[1]); const RELEASE = process.argv[2];',
'const mine = fs.readdirSync("/proc/self/fd").filter((n) => {',
' try { return fs.readlinkSync("/proc/self/fd/" + n) === LOCK; } catch { return false; }',
'}).length;',
'const helper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1e9)"], { stdio: "ignore" });',
'fs.writeSync(1, "CHILD_LOCKFDS=" + mine + "\\nHELPER=" + helper.pid + "\\nEXECED=1\\n");',
'setInterval(() => { if (fs.existsSync(RELEASE)) process.exit(0); }, 25);',
'setTimeout(() => process.exit(0), 60000);',
].join('');

fs.writeFileSync(
Expand All @@ -87,32 +122,70 @@ describe.skipIf(!RUNNABLE)('gen-sdui-manifest.sh cleanup contract', () => {
// Sourcing runs no generation — the script returns right after defining
// its lifecycle helpers, so this exercises the REAL functions.
`source ${JSON.stringify(SCRIPT)}`,
`DEVLOG=${JSON.stringify(devlog)}`,
`RELEASE=${JSON.stringify(release)}`,
`LOCK=${JSON.stringify(lock)}`,
`LOCKBASE=${JSON.stringify(path.basename(lock))}`,
'DUMP_PID_FILE="$(mktemp "${TMPDIR:-/tmp}/sdui-dump-pid.XXXXXX")"',
`trap 'sdui_stop_detached "$DUMP_PID_FILE"' EXIT`,
`sdui_spawn_detached "$DUMP_PID_FILE" ${JSON.stringify(path.join(dir, 'dev.log'))} \\`,
` "$(command -v node)" -e ${JSON.stringify(stub)}`,
'sdui_spawn_detached "$DUMP_PID_FILE" "$DEVLOG" \\',
` "$(command -v node)" -e ${JSON.stringify(stub)} "$LOCK" "$RELEASE"`,
'LEADER="$(cat "$DUMP_PID_FILE")"',
'echo "LEADER=$LEADER"',
'echo "LEADER_SID=$(ps -o sid= -p "$LEADER" | tr -d " ")"',
'echo "SCRIPT_PGID=$(ps -o pgid= -p $$ | tr -d " ")"',
// Count inherited descriptors NOW, while the session LEADER is still
// alive. Counting later would inspect only the reparented helper, which
// never inherits the fd anyway (node closes it across its own spawn) —
// an assertion that stays green with the fd hygiene deleted. That green
// was observed before this line moved, so the ordering is the test.
// Barrier 1 — the spawned command's own post-exec report. NOT the
// pidfile: see the note on property (2) in this file's header. This
// waits for the reading to become TAKEABLE, never for it to become
// zero.
'CHILD_REPORTED=no',
'for _i in $(seq 1 600); do',
' if grep -q "^EXECED=1$" "$DEVLOG" 2>/dev/null; then CHILD_REPORTED=yes; break; fi',
' sleep 0.05',
'done',
'echo "CHILD_REPORTED=$CHILD_REPORTED"',
'CHILD_LOCKFDS="$(sed -n "s/^CHILD_LOCKFDS=//p" "$DEVLOG" 2>/dev/null || true)"',
'HELPER="$(sed -n "s/^HELPER=//p" "$DEVLOG" 2>/dev/null || true)"',
'echo "CHILD_LOCKFDS=$CHILD_LOCKFDS"',
'echo "HELPER=$HELPER"',
// Barrier 2 — the helper has joined the session, so the session-wide
// count below covers both members instead of whoever happened to exist.
'HELPER_IN_SESSION=no',
'for _i in $(seq 1 200); do',
' MEMB="$(pgrep -s "$LEADER" 2>/dev/null | tr "\\n" " " || true)"',
' case " $MEMB " in *" $HELPER "*) HELPER_IN_SESSION=yes ;; esac',
' if [ "$HELPER_IN_SESSION" = yes ]; then break; fi',
' sleep 0.05',
'done',
'echo "HELPER_IN_SESSION=$HELPER_IN_SESSION"',
// The session-wide count, now ordered after both barriers. Naming the
// holders turns a future red from "1, expected 0" into a process.
'FDS=0',
'HOLDERS=""',
'for p in $(pgrep -s "$LEADER"); do',
` n=$(ls -l "/proc/$p/fd" 2>/dev/null | grep -c ${JSON.stringify(path.basename(lock))} || true)`,
' n=$(ls -l "/proc/$p/fd" 2>/dev/null | grep -c "$LOCKBASE" || true)',
' if [ "$n" != 0 ]; then HOLDERS="$HOLDERS $p($(tr -d "\\0\\n" < "/proc/$p/comm" 2>/dev/null || echo unknown))"; fi',
' FDS=$((FDS + n))',
'done',
'echo "LOCKFDS=$FDS"',
'echo "LOCKFD_HOLDERS=[$HOLDERS]"',
`echo "LEADER_ALIVE_AT_COUNT=$(kill -0 "$LEADER" 2>/dev/null && echo yes || echo no)"`,
// Now let the stub exit, so its helper is reparented to init before cleanup.
'sleep 2',
'MEMBERS="$(pgrep -s "$LEADER" | tr "\\n" " ")"',
'echo "MEMBERS=$MEMBERS"',
// Now release the stub, so its helper is reparented to init before
// cleanup, and wait for that to have HAPPENED rather than sleeping a
// guessed interval. A timeout leaves ORPHAN_BEFORE empty, which the
// precondition assertion reads as a red.
': > "$RELEASE"',
'ORPHAN=""',
'for p in $MEMBERS; do [ "$(ps -o ppid= -p "$p" | tr -d " ")" = "1" ] && ORPHAN="$p"; done',
'MEMBERS=""',
'for _i in $(seq 1 400); do',
' MEMBERS="$(pgrep -s "$LEADER" 2>/dev/null | tr "\\n" " " || true)"',
' for p in $MEMBERS; do',
' if [ "$(ps -o ppid= -p "$p" 2>/dev/null | tr -d " ")" = "1" ]; then ORPHAN="$p"; fi',
' done',
' if [ -n "$ORPHAN" ]; then break; fi',
' sleep 0.05',
'done',
'echo "MEMBERS=$MEMBERS"',
'echo "ORPHAN_BEFORE=$ORPHAN"',
// Falling off the end fires the EXIT trap, which is the path under test.
].join('\n'),
Expand All @@ -133,13 +206,20 @@ describe.skipIf(!RUNNABLE)('gen-sdui-manifest.sh cleanup contract', () => {
expect(field('LEADER_SID'), out).toBe(leader);
expect(field('SCRIPT_PGID'), out).not.toBe(leader);

// The barriers themselves: a timed-out barrier must arrive as a named red,
// not as a count taken at some other moment.
expect(field('CHILD_REPORTED'), out).toBe('yes');
expect(field('HELPER_IN_SESSION'), out).toBe('yes');

// (3, precondition) the orphan the naive `kill "$!"` cannot reach really
// existed. Without this, the survivor assertion below could pass vacuously.
expect(field('ORPHAN_BEFORE'), out).toMatch(/^\d+$/);

// (2) nothing in the session inherited the caller's lock descriptor —
// counted while the leader was still alive, which is the only moment the
// count can be non-zero and therefore the only moment it means anything.
// (2) nothing in the session inherited the caller's lock descriptor. Two
// distinct facts: the surviving-capable process's reading of ITSELF, taken
// after `exec` and therefore after the close; and the session-wide count,
// taken while the leader was still alive and after both barriers.
expect(field('CHILD_LOCKFDS'), out).toBe('0');
expect(field('LEADER_ALIVE_AT_COUNT'), out).toBe('yes');
expect(field('LOCKFDS'), out).toBe('0');

Expand Down
Loading