Repository navigation
test(harness): single descendant cleanup, strict absence proof - #2687
Conversation
macOS bash 3.2 reports -c input as line 0, modern GNU bash as line 1. Compare diagnostic shape with line number normalized.
…gression A long -c ARG0 stamped on every report line could evict the structural payload out of the 1 KiB diagnostic budget. Truncate the shell name to 64 chars first so 'syntax error ...' always survives; renumber the threat entry to TM-DOS-136 (main already owns 135 via #2674); add a bounded-child regression exercising the exact finding shape (multiline token x long ARG0) at two scales.
The 64-char cut mangled long script paths (breaking the oracle's path
replacement and hiding which script failed). Widen to 256 chars with a
16-char tail: realistic paths print verbatim with attribution suffixes
(': eval') intact, while a kilobyte-scale ARG0 still cannot evict the
structural payload out of the 1 KiB total. The total stays the enforced
bound.
The scaled multiline-token x long-ARG0 regression now asserts the whole harness contract at both scales (200/2000 lines x 8 KiB ARG0): no watchdog timeout, raw child exit 2, no output-cap overshoot, finished reader threads, and effective stack=4096/cpu=10 caps read back from the launcher. Shape and 1 KiB assertions unchanged.
run_command_bounded cleanup per lifecycle review: truncate retention before extending (retention never passes the 8192 cap); retry Interrupted, fail closed with partial bytes on real reader errors (new read_error field, never relabeled as EOF); exact-cap EOF is not excess; reap inside a finite 5s grace, never an unbounded wait; timed_out retained as recorded; drain 2s + reap 5s graces documented against the execution deadline. Consumer regression gains !read_error and the corrected 8192-byte cap text. Bounded selftests: exact-cap, cap+1, Interrupted/error scripted pipes, timeout kill+reap, overflow flag.
Per lifecycle review on #2682: join readers only after all channel receipts (receipt proves all blocking work done; missing receipts skip the join and fail incomplete), with panicking readers failing visibly; reap extracted into a poll seam with deterministic verdict unit tests; timeout/overflow selftests run under the fail-closed launcher with stack/cpu readback and elapsed-total bounds; retained-pipe case gains elapsed proof and read_error distinction. Reuses existing bounded_helper_flags_retained_pipes; does not touch #2681's distinct probe helper.
…tests Per completion review on #2683: join readers only while is_finished inside the shared completion grace (a receipt is not the termination contract); deterministic paused-after-send proof with release/cleanup watchdog; poll/read/reap faults injected through the real parent lifecycle with distinct fail-closed verdicts (no panicking expect on child poll); timeout/overflow selftests under the fail-closed launcher with cap readback and elapsed bounds; existing retained-pipe fixture reused and strengthened. Untouched: #2681 distinct probe helper.
…tests Per completion review on #2683: join readers only while is_finished inside the shared completion grace (a receipt is not the termination contract); deterministic paused-after-send proof with release/cleanup watchdog; poll/read/reap faults injected through the real parent lifecycle with distinct fail-closed verdicts (no panicking expect on child poll); timeout/overflow selftests under the fail-closed launcher with cap readback and elapsed bounds; existing retained-pipe fixture reused and strengthened. Untouched: #2681 distinct probe helper.
… selftests Per completion review on #2684: common cleanup termination funnels every loop exit through one reachable kill guard; bounded is_finished join gate with shared completion grace (receipt is not the termination contract); deterministic paused-after-send proof drives the actual gate with bounded readiness and release/cleanup watchdog; poll/read/reap faults injected through the real parent lifecycle with distinct fail-closed verdicts (no panicking child poll); selftests and retained-pipe fixture under the fail-closed launcher with cap readback and elapsed bounds. Reuses existing retained-pipe fixture; untouched: #2681 distinct probe helper.
…receipts Per completion review on #2685: join_finished returns the unfinished handle so the paused-after-send proof releases and joins THAT SAME thread (no twin); poll-fault fixture uses direct-exec sleeper so killing closes the pipes with no orphan; reap_never keeps real bounded OS cleanup while the verdict reports None; retained-pipe fixture runs under the fail-closed launcher with a file side-channel receipt proving the effective bounds despite incomplete pipe capture. Reuses the existing retained-pipe fixture; untouched: #2681 distinct probe helper.
Per completion review on #2685: fail_poll fires only after a readiness file proves the victim truly live (direct-exec sleeper, no orphan); BoundedRun gains a reaped receipt separating actual OS cleanup from exit/observation verdicts; retained-pipe fixture runs under the fail-closed launcher with a file side-channel receipt proving the effective bounds despite incomplete pipe capture. Reuses the existing retained-pipe fixture; untouched: #2681 distinct probe helper.
The retained-pipe fixture routes through the fail-closed launcher with a file side-channel receipt; the leftover comment claiming a direct run contradicted the code. No behavior change.
…ture The retained-pipe fixture retains its sleeper's PID through a receipt file, then terminates that exact descendant and finitely observes its completion (kill -0 absence polling inside a 5s bound) after asserting the negative verdict, with a total end-to-end elapsed bound. No waiting for the 30s self-expiry, no universal process-tree reaper, no change to the truthful incomplete flags.
The retained-pipe fixture contained two cleanup blocks for the same recorded descendant, so the second kill correctly failed with No such process. Consolidate cleanup to exactly one path after the negative verdict: terminate the retained descendant, join the original reader handles, and prove absence with kill -0 where only an observed non-zero exit counts (spawn errors stay unknown inside the finite bound). No behavior change to the verdict.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | b075cf4 | Commit Preview URL Branch Preview URL |
Oct 11 2026, 04:46 PM |
|
Focused follow-up to full review #2686 (comment), against head 19b13e5. The duplicate cleanup is now genuinely consolidated, and PendingJoiners retains and finitely joins the ORIGINAL reader handles. Preserve those repairs and the truthful incomplete-reader verdict, effective bounds, readiness, and actual OS reap assertions. The remaining test-scoped cleanup contract is not yet fail-closed: threat_model_tests.rs:1144-1148 and :1167 synchronously wait on external kill commands without a per-command deadline. The surrounding elapsed assertion/loop cannot bound a blocked status() call. Also, every nonzero kill -0 result is treated as absence at :1169, although permission/other errors are not an ESRCH receipt. Finally, negative-verdict/bounds assertions at :1123-1137 occur before cleanup, so a failed assertion drops the original handles and leaves the owned sleeper until self-expiry. Please use existing structured signaling/error APIs where available (explicit ESRCH versus EPERM/unknown) or an equivalently bounded helper; retain one owned cleanup guard/path that also runs on setup/assertion failures, and finitely observe the original handles. Update the stale 30-second outer-deadline comment. Exercise the cleanup error/unwind boundary without weakening negative flags or substituting twin handles. No demonstrated hang or universal production process-tree requirement is claimed. Read full conversation comments as well as formal/inline reviews before merge, and capture actual cargo producer exits rather than cargo|tail pipeline exits. The locally filtered misconfig skip remains partial; unchanged required Linux CI and exact-merged original/parity assertion proof with live output caps are still required. |
The retained-pipe fixture builds a RetainedCleanup guard owning the exact descendant PID, the ORIGINAL reader handles, and both receipt files before any verdict assert, so setup/assertion failures still clean up via Drop (best-effort kill, no panics). The normal path terminates the descendant, joins the original handles through the shared gate, observes ESRCH absence (spawn errors stay unknown), and proves the total elapsed bound. Only an observed non-zero exit counts as absence.
|
Follow-up against additive head 161f750, preserving the genuine ORIGINAL-reader ownership, consolidated cleanup, and explicit unknown-versus-no-such-process improvements from review6110620716. The new signal_bounded helper still does not own completion on every return. Its grace-expiry and try_wait-error arms return Unknown without killing/reaping the helper child. After successful polling it synchronously read_to_string's the stderr pipe without a deadline/output cap, then calls wait; successful try_wait already reaped the direct child, but it does not establish pipe EOF if another process retains the pipe. Please use the existing reviewed bounded process/reader machinery or equivalent finite owned completion, and keep errors UNKNOWN rather than successful absence. A deadline around try_wait alone is not an end-to-end bound. No actual hang or leaked helper has been demonstrated. RetainedCleanup is constructed only after two fallible receipt reads. A failure there still drops pending original readers before the guard exists. Drop only best-effort signals and detaches pending handles, with no finite completion receipt; it cannot justify the comment that setup/assertion failures never leak. Establish ownership before fallible setup, retain true negative verdicts, and exercise the actual assertion/setup/helper-error cleanup boundary with bounded original-handle completion. Update the still-stale 30-second outer-deadline comment. No universal production process-tree contract is requested. Keep this on existing PR2687 with normal additive pushes, all history/unrelated files intact. Read complete comments/formal/inline reviews. Capture actual cargo exit rather than cargo|tail; 222 filtered tests with misconfig skip are partial, not an unchanged Linux-CI waiver. Do not merge on CI alone before resolving the focused cleanup contract and then running exact-merged bounded original/parity assertion proof with live file caps. |
Per ownership review on #2687: signal_bounded reuses drain_capped and reap_with_grace so the helper child is terminated and reaped on every path and no return leaks a running process; EOF reasoning is construction-specific to the non-forking kill binary. The retained fixture builds its cleanup guard immediately after the run, before any fallible setup read, so panics in reads or verdict asserts still clean up via Drop; the pid binds once the pidfile proves identity. Only an observed non-zero exit counts as absence.
1 participant
What changed
Test-only, one file: threat_model_tests.rs. Single descendant cleanup in the retained-pipe fixture plus strict absence proof:
Before / After
Before: duplicate kill of an already-reaped descendant failed the suite.
After: threat_model 222 passed (only pre-existing macOS nesting abort skipped, identical on unmodified code); error 14, partial_parse 11 green; fmt plus clippy -D warnings clean.
Risk
Checklist
Update: guard-owned cleanup with unwind coverage
Follow-up commits on the same branch add:
Update: reworked signal helper, guard-first ownership
Follow-up commit on the same branch: