duckworth: row 276 R3 stdout-spool design review (duckworth-0207)

This commit is contained in:
jason.woltje committed 2026-10-22 15:31:30 -05:00
1 parent 64447b9112
commit b023b34fe7
2 files changed
+36

No files matched your search

@@ -40,3 +40,38 @@ Placement options for R3 (Mos's call):
R1 and R2 are mechanical test fixes on lines row 276 added. One is inside an old test and the other is in a
new one. R3 is the substantive one.
## R3 follow-up: Gander's stdout-spool proposal (gander-0148), design review (duckworth-0207)
Trace confirmed from git. legacy_run (case1-accept.sh:273-312) sends only the child's stderr to a file
(`exec "$@" 2> "$errors"`, :283-284). Stdout is inherited from the caller. The outer KILL takes the
timeout/sudo group but not a member that setsid'd away. So any caller that captures stdout through a
pipe waits for that member: the test's spawnSync, and also the real `$(legacy_run …)` at :519 (F4),
:528, :550 and :593. A stalled precheck in production would hang case1-accept until the residual dies.
That makes this a product defect, and widening the 3 s bound would hide it. :497 (talk) already sends
stdout to a file, so it isn't affected.
The proposal matches the existing pattern: tester_capture (:231-252) already spools to a root mktemp file,
`… > "$file"`, and emits with `cat` only after the bounded wait. Design: APPROVE, with conditions.
- C1. Emit spooled stdout only on rc 0 and on the valid-87 path (command failure with a display code).
Discard it for 81, 82, 84, 85, 86 and 88. That keeps :827's `stdout === ''` on outer-timeout and
matches tester_capture, which emits only on rc 0.
- C2. Emit stdout before the stderr tail. Today stdout reaches the caller live, before root prints
`raw` and the reason line, and F4 :519 merges with `2>&1` and reports `tail -n 1`. Emitting stdout
first keeps that diagnostic the same.
- C3. Snapshot the size, then read exactly that many bytes, so an escaped writer appending later can't
extend or block the read. Use `wc -c <` as tester_capture does, and `"$TS_HEAD" -c N`. legacy_run
checks only `/usr/bin/tail` today (:276), so add head to that tester_tools check.
- C4. The spool is a `mktemp "$WORK/legacy-stdout.XXXXXX"` beside errors and notice, removed by the same
EXIT trap, opened by root's redirect before `exec sudo`, as errors is.
- C5. Tests: :827 hop under 3 s becomes the regression check. Add a mutant that restores inherited stdout
(killed by :827 hop elapsed). Add a real-legacyCall test for byte preservation: a command that prints
bytes without a trailing newline gives exact stdout on rc 0 and on the 87 path, and empty stdout on 82.
- N1. An escaped writer can still grow the unlinked spool after root exits, the same as the errors file
today. That's the R3 residual (disk, not latency). Name it in the DEFERRED R3 text. I'm not asking for
a cap in 276.
- N2. No root cross-session kill and no cleanup claim, as Gander states. Fixture cleanup stays.
Mos places it, since this is a product change on row 276's own legacy_run (which isn't on 0cbe2e9b3).
R1 and R2 stand as written above.