Fix the capture and session timeout hang - #343
Conversation
A killed child that left a descendant holding the inherited stdout/stderr pipe blocked the drain-thread joins forever: read_to_end returns only at pipe EOF, so Outcome::TimedOut never came back. Reachable in production via git fetch over ssh (five capture-with-timeout verbs in clients/git.rs). The drains are now abandoned on expiry rather than joined: a timed-out outcome drops its output by contract, and a setsid'd descendant (ssh ControlMaster) survives any kill aimed at the child, so the join can never be the liveness guarantee. First slice of the kill strategy decided on #301; refs #302.
Abandoning the drains (previous slice) made the timeout live but leaked the tree: the single-pid SIGKILL left a grandchild running and the pipe open. Captures now lead a process group of their own — nothing they run reads the terminal, so the SIGTTIN hazard that keeps interactive children in dl's group does not apply — and the expiry kill is a killpg on that group, with the single-pid kill kept as the fork-to-exec-window fallback. Second slice of #301's decided shape; refs #302.
session shared the capture hang through its one pipe: after the expiry kill, reader.join() blocked until stderr hit EOF, which a setsid'd descendant holding the fd postpones forever. The join now happens only when the pipe closed of its own accord (the reader thread is already exiting); on a timeout it is abandoned. The group kill does not apply here — session's child must stay in this process's group or an interactive child takes SIGTTIN. Third slice of #301's decided shape; refs #302.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideMakes capture/session timeouts non-hanging by abandoning drain joins on timeout and killing whole process groups for captures, while keeping session children in the parent group for SIGTTIN safety, and adds tests for the ssh/ControlMaster-style descendant pipe holders. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
blooop
left a comment
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Reviewed at merge-base a1c6fe8 → 00e24cc. Checks green (ci, rust, e2e, public-api, packaging, prek, gate, codecov); cargo test -p devlaunch-runner 38/38 on the head. Every finding below was reproduced by A/B — the same example binary built twice against the same tree with only devlaunch-runner/src/lib.rs swapped.
Standards
S1 — blocking. lib.rs:421-426: a capture that reads the terminal is now SIGTTIN-stopped, and for the untimed captures that is a permanent hang.
The comment asserts "Nothing here reads the terminal, so the SIGTTIN hazard that keeps an interactive child in this process's group does not apply." That premise is false. SpawnSpec::stdin defaults to StdinPlan::Inherit (lib.rs:192), and a child moved out of the foreground group takes SIGTTIN on any read of the controlling terminal — its own stdin or /dev/tty, which is where ssh's host-key confirmation, ssh's passphrase prompt and git's credential prompt all read from. try_wait does not report a stopped child (no WUNTRACED), so nothing notices.
A/B, same example, only lib.rs differing, run under a pty with yes\n on stdin, capturing sh -c "printf 'prompt: ' >&2; head -c 3 /dev/tty":
main: capture returned after 1.787102ms: Ran { exit: Code(0), io: { stdout: "yes\n", stderr: "prompt: " } }
#343: (killed at 10s — never returned) child state in /proc: T (stopped), tcpgrp != its pgid
Three captures reach this with no timeout at all, so there is no escape: git clone --bare (clients/git.rs:369), git push -u (:606), and the launch-path git fetch (:388, fetch_all(bare, None)). First clone of an ssh remote with no known_hosts entry, or any passphrase-protected key without an agent, now wedges dl forever. The timed ones degrade instead: ls_remote_symref_head uses git@github.com:… (repo_manager.rs:1655) and would now burn its 10 s and report "git ls-remote timed out" where the user used to be prompted.
CI cannot see this — no controlling terminal.
S2 — blocking. lib.rs:421-426 + interrupt.rs:131: Ctrl-C no longer reaches a capture child, and nothing else kills it.
Leaving dl's group also leaves the terminal's foreground group, so a terminal SIGINT is delivered to dl alone. Unlike passthrough, capture never calls note_foreground_child, so cleanup_and_exit's killpg has nothing to aim at — and dl's disposition is _exit(130) (dl/src/lib.rs:110-121), which does not wait. The child is orphaned outright.
A/B, real Ctrl-C (\003 into the pty), capture of sh -c 'exec sleep N':
main: script exit=130 — child gone: Ctrl-C reached it
#343: script exit=130 — ORPHANED: sleep survived Ctrl-C
This re-opens the F3 orphan class interrupt.rs's module doc exists to close, and it contradicts a documented contract the fetch depends on: "A launch is watched and interruptible, so it passes None" (repo_manager.rs:1385). That fetch runs under the repo flock, which _exit releases — so after a Ctrl-C an unsignalled git fetch keeps writing the bare cache with no lock held.
Non-blocking
lib.rs:821,:855—own_groupis threaded as a parameter parallel to a fact already established instart(). Five hand-paired call sites; a mis-pairkillpgsdlitself, and only prose prevents it. ReturnSpawned { child, group }fromstartand the invariant cannot be restated wrong. (ThekillpgSAFETY comment at:857asserts a property of the caller's argument that the signature cannot guarantee; same fix.) The group kill itself is sound — Linux pins astruct pidwhile it is a live pgid, so the child's pid cannot be recycled under thekillpg; verified by exhausting 200k pids against a leader-exited group.lib.rs:430-437vs:487-495— the parent-sidesetpgidblock and its comment are now duplicated verbatim, and have already drifted ("and" vs "or"). Belongs once insidestartunderOwnGroup::Yes.passthrough'sown_groupbranch (lib.rs:497) silently gained group-kill-on-timeout. Currently inert (devpod upsets no timeout) and arguably right, but undecided and untested.- Doc drift:
OwnGroup(:703-709) still says the group exists only for the interrupt handler;SpawnSpec::own_group(:221-231) says "Onlypassthroughreads this field" — true, yet a capture spec withown_group: falsenow leads its own group anyway. - Tests:
tests.rs:338,:482do not assertsetsidsucceeded — absent, the script still runs and both go green with no grandchild. The poll-with-deadline scaffolding is duplicated verbatim between them. Nothing pins the dangerous direction (that a non-own-group child is not group-killed), and no test pins capture's group placement, whichpassthroughhas both ways (:599,:623).
Spec
Measured against #301's human-decided comment and #302's agreed seams.
P1 — satisfied. "Captures spawn in their own process group and the expiry kill becomes a killpg." Delivered at lib.rs:426, :436, :855-866, mirroring the existing passthrough pattern.
P2 — blocking, half-delivered. "the drain-thread joins are additionally time-bounded and abandoned on expiry, because a setsid'd descendant … escapes any group kill — the liveness guarantee cannot rest on the group."
Only "abandoned on expiry" landed. collect() moved into the Ending::Ended arm (lib.rs:448-449), so the success path still joins unbounded — and expiry never happens there, so abandonment cannot help. The result is that the hang the ticket exists to fix survives on the more common path:
sh -c "setsid sleep 30 & printf done", timeout 200ms, on #343 head
→ never returned (killed at 15s)
The child exits 0, wait returns Ended, the setsid'd descendant holds the stdout pipe, collect() blocks forever. This is precisely the shape #301 names — "ssh ControlMaster is the production example" — and a git fetch that succeeds while leaving a master behind is at least as reachable as one that times out. A bound on the join, as decided, would have caught both; the abandon-only reading catches one. Both collect() calls need a deadline.
P3 — non-blocking. "session's stderr reader.join() … gets the same bound." lib.rs:591 gives it the same abandon, not a bound. Adequate here — with a timeout set, the recv_timeout loop always sets timed_out before the join, so session cannot reach capture's success-path shape — but the word "bound" is unmet and the asymmetry is worth a line.
P4 — satisfied. All three seams named in #302's breadcrumb are present (tests.rs:338, :364, :482), named exactly as agreed, and genuinely red: merge-base lib.rs + this PR's tests.rs fails 3/3, each with the message its name promises.
P5 — the spec's premise, not the implementation. #301 licensed the group move on the reasoning that captures never read the terminal. S1 refutes that by measurement. The PR implements the decision faithfully; the decision itself needs revisiting, so this is not a builder error. Note also that the spec did not weigh S2 — Ctrl-C reachability was never part of the trade it considered.
No other scope creep: the wait(…, own_group) threading and the doc edits are the minimum needed to carry the decision.
Verdict
Request changes (posted as a comment — GitHub refuses --request-changes on one's own PR).
Blocking:
- S1 — capture children in their own group take SIGTTIN on any terminal read; permanent hang for
clone_bare,push_branchand the launch fetch, all of which pass no timeout. - S2 — Ctrl-C no longer reaches a capture child and nothing else kills it; the launch fetch is orphaned holding no lock, re-opening F3.
- P2 — the success-path
collect()join is still unbounded, so the ticket's own hang survives on the commoner path.
S1 and S2 both follow from half (a) of #301, whose stated premise measurement refutes — so this likely wants #301 reopened rather than a straight rework. Two shapes worth weighing there: make the group opt-in per spec (SpawnSpec::own_group already exists; set it on the internal timeout-bearing verbs that provably cannot prompt) and note_foreground_child the capture group for the duration of the wait, which answers S2 either way.
P2 is independent of that decision and is a straight fix: bound both collect() joins.
Drops the process-group half of #301's decision, which the human reversed after PR #343's review A/B-measured it. Two defects, both refuting the premise the killpg rested on ("nothing a capture runs reads the terminal"): - SIGTTIN. `SpawnSpec::stdin` defaults to Inherit and `/dev/tty` is reachable whatever stdin is, so ssh's host-key confirmation, ssh's passphrase prompt and git's credential prompt all read the terminal from inside a capture. A child outside the foreground process group is stopped rather than served, and `try_wait` never reports a stopped child. Measured under a pty: main returns in 1.8 ms with the prompt served; the process-group build never returns, child in state T. Three captures pass no timeout at all (`git clone --bare`, `git push -u`, the launch-path fetch), so for those there is no deadline to end the hang. - Ctrl-C. Leaving dl's group leaves the terminal's foreground group, and `capture` never notes a foreground child for the interrupt handler to killpg, so dl's `_exit(130)` reaches nothing. The launch fetch is then orphaned writing the bare cache with the repo flock already released — the F3 orphan class reopened. Both are now pinned by tests, red against the reverted code and green here: `tests/terminal.rs` drives `examples/terminal_capture.rs` through a real pty (the only place a controlling terminal exists), typing at a capture that reads `/dev/tty` and Ctrl-C'ing one that would outlive it. What this gives up knowingly: a timed-out capture may leave what the tool forked running, exactly as main does today. The tree-killing was the lesser concern. `wait`/`kill` lose the `own_group` parameter with it, and the group-kill test goes with the behaviour it pinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…exit The headline hang survived on the commoner path. The drain joins were made abandonable on the timeout path only, so a child that exits 0 while a descendant it forked into a session of its own still holds the stdout pipe left `collect()` blocked on an EOF that never comes — the ssh ControlMaster shape #301 names, on the path a `git fetch` reaches far more often than a timeout. Measured on the previous head: `sh -c "setsid sleep 30 & printf done"` never returned. So the join is bounded rather than abandoned, which is what #301's decision said and what its reversal leaves as the whole of the fix. The bound is a fixed grace, not the remaining timeout: by the time it is waited on the child is gone, so everything it wrote is already read or sitting in the pipe buffer, and the wait is only ever for the pipe to close. That covers the three captures that pass no timeout at all — `git clone --bare`, `git push -u`, the launch-path fetch — where a deadline could never have helped. The bytes therefore live behind a lock rather than in the drain thread's return value, so what was read can be taken without joining the thread that read it: a bounded join that returned an empty string would turn the hang into a silently wrong answer for the callers that parse stdout. `tests.rs` gains the success-path test (red before this, hanging at its own 5 s bound) and, for the reviewer's non-blocking finding, a `setsid` check — without the tool the grandchild tests still ran and pinned nothing — plus one shared piece of hang-bounding scaffolding instead of three copies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reworked on S1 — capture children take SIGTTIN on a terminal read. Fixed by dropping the process-group half. Reproduced first, and it needed the pty the review used: S2 — Ctrl-C no longer reaches a capture child. Fixed by the same revert, and pinned separately. The revert commit message carries both measurements. What it knowingly gives up is in P2 — the success-path join was still unbounded. Fixed, and this is now the whole of the fix. Two choices in there worth flagging, both narrower than "add a deadline":
P3 — the session/capture asymmetry. Left as an abandon, as the review judged adequate, but the comment now says what it is: a bound of zero, and why zero is right on both paths that take it (a timed-out outcome carries no text, and session's lines were already handed over as they arrived). Non-blocking, also addressed: the duplicated Not addressed: Full workspace suite green locally ( |
The coverage job runs `cargo llvm-cov`, which spawns `cargo test --tests` — and that builds no examples, so the helper the terminal tests drive was absent there and both failed on the assertion that says so. A helper that can go missing under one of the two CI runs is the wrong shape; this binary re-executed on the pty (`--exact <test> --nocapture`, with the role in an environment variable) cannot. Still red against the build it was written to refute: with capture back in a group of its own, `a_captured_child_may_read_the_terminal` times out with the pty showing only the echo, and `a_terminal_interrupt_reaches_a_captured_child` finds the marker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
One correction to the reply above, and the reason: the pty helper is no longer a separate The example shape failed All checks green on |
blooop
left a comment
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Re-reviewed at 6a2579e (fast-forward from the 00e24cc I reviewed; 00e24cc confirmed an ancestor, nothing rebased away). All three blocking findings resolved. Everything below is my own measurement, not the summary I was handed.
S1 / S2 — resolved by reversion, and nothing of the group change survives
Verified structurally rather than by reading the revert commit. Net of the whole PR (a1c6fe8..6a2579e), the only lines matching OwnGroup|killpg|setpgid|fn wait|fn kill are two doc-comment lines. capture spawns OwnGroup::No (lib.rs:435); the parent-side setpgid is gone; wait/kill are back to their merge-base signatures; the sole remaining OwnGroup::Yes is passthrough's own_group branch, exactly as on main. The killpg scope creep I flagged on passthrough's timeout path went with it.
The two new pty tests are sensitive. I did not take the builder's word for this. Flipping the single value at lib.rs:435 back to OwnGroup::Yes and rerunning --test terminal:
test a_terminal_interrupt_reaches_a_captured_child ... FAILED
the captured child outlived the terminal's Ctrl-C: nothing else kills it
test a_captured_child_may_read_the_terminal ... FAILED
"outcome: " never appeared: the capture never returned — a child outside the
terminal's foreground process group takes SIGTTIN on a terminal read
Both red, each with the message its own name promises, in 30 s. Restored: 38 unit + 2 pty green, clippy -D warnings and fmt --check clean.
This is a better guard than I asked for. My S1/S2 were measured with a throwaway example binary; these pin the cause — group membership — inside cargo test, so the dangerous direction cannot be reintroduced silently. Re-executing the test binary under DEVLAUNCH_TEST_TERMINAL_ROLE rather than shipping an example is the right call for the stated reason: cargo llvm-cov runs cargo test --tests and builds no examples, so an example-based helper would be absent from one of the two CI runs.
P2 — resolved, and the chosen fix is better than the bound I asked for
The new test is genuinely red against the head I reviewed. Merge-base-plus-one lib.rs (00e24cc) with this PR's tests.rs:
---- a_capture_returns_when_a_grandchild_holds_the_pipe_past_a_clean_exit ----
capture never returned: the success path is waiting on a pipe a grandchild still holds
I probed the three things worth doubting about Drain/DRAIN_GRACE:
| probe | result |
|---|---|
| 1 MiB on stdout, exit 0, setsid'd holder | stdout_len=1048576, 1.02 s — intact, not truncated |
| both pipes held | stdout="out" stderr="err", 1.01 s |
| no holder (the normal path) | "out"/"err", 8 ms — grace skipped entirely |
That answers the truncation worry. The reader thread runs concurrently for the child's whole life, so its backlog at child-exit is bounded by pipe capacity (~64 KiB), not by payload size — the 1 MiB probe confirms 500 ms covers it with room to spare. And because collect short-circuits on is_finished(), the grace is paid only when a descendant holds the pipe — i.e. only in the case that previously hung forever. Strictly better, never worse.
The fixed grace is more defensible than the deadline-derived bound I asked for, and I withdraw that part of my finding. Two reasons: the three untimed captures would derive None and hang, which is the defect; and once the child has exited the remaining timeout is the wrong quantity — it bounds how long the tool may run, not how long its pipe takes to close, so a capture with 100 ms left of a 30 s budget would truncate output it already had in hand. Keeping the bytes behind a Mutex so they can be taken without joining is the right shape: it avoids converting the hang into a silently-empty stdout for callers that parse it. Poisoning is handled by held() and is unreachable anyway (the thread only extends a Vec under that lock).
One new non-blocking defect the rework introduces
lib.rs:816-826 — the hand-rolled read loop drops the Interrupted retry read_to_end gave for free. Ok(0) | Err(_) => break treats ErrorKind::Interrupted as EOF. Proved with a reader that yields "ab", one Interrupted, then "cd":
read_to_end (the code this replaced): "abcd"
hand-rolled loop (#343): "ab"
The failure mode is silent truncation reported as Outcome::Ran with a success exit — precisely the silently-wrong-answer harm cited as the reason not to return String::new(). Not reachable today: the workspace installs exactly one handler, libc::signal(SIGINT, …) in dl/src/lib.rs:120, and glibc's signal() carries BSD semantics with SA_RESTART, so nothing here generates EINTR on a pipe read. But it is a robustness regression against the previous behaviour and the fix is one arm: Err(e) if e.kind() == ErrorKind::Interrupted => continue.
Two nits, neither worth a commit on its own: capture pays up to 2 × DRAIN_GRACE serially (measured 1.01 s) where DRAIN_GRACE's doc reads as a single bound on the call; and session's join is still an abandon rather than a bound (my earlier P3) — correct as written, since the recv_timeout loop always sets timed_out before the join, but the comment's "a bound of zero" is doing some work.
The accepted regression — documented, not tested: I accept it
I verified the "not a regression" claim rather than relaying it. Same probe ((sleep 1; : > marker) & exec sleep 30, 100 ms timeout) against both trees:
new head 6a2579e: forked descendant survived = true
merge-base a1c6fe8: forked descendant survived = true
Identical, so kill's doc comment is accurate: this is main's behaviour, not something the PR introduces, and a test would pin pre-existing default behaviour rather than anything this ticket changed. More to the point, the thing actually worth guarding is guarded: the risk with a knowingly-accepted trade is that someone later reintroduces the killpg to fix it and silently re-breaks S1/S2 — and tests/terminal.rs fails loudly on exactly that, as the sensitivity run above shows. Pinning the cause beats pinning one consequence.
My four earlier non-blocking Standards items are all gone too: the parallel own_group parameter, the duplicated setpgid block, the passthrough scope creep, and the missing setsid guard plus duplicated poll scaffolding (now require_setsid() and within()).
Verdict
Approve — posted as a comment, since GitHub refuses --approve on one's own PR.
All three blocking findings resolved: S1 and S2 by full reversion with the dangerous direction now pinned by sensitive pty tests, P2 by a bounded, byte-preserving drain that I measured intact at 1 MiB and free on the normal path. The one new defect (Interrupted treated as EOF) is non-blocking and unreachable in this workspace today; worth a one-line follow-up, not a gate. Merging stays a human call.
The bound `collect` puts on a drained pipe is the whole of this fix, and it was right. How it waited was not: `JoinHandle::is_finished` polled at `POLL_INTERVAL` charged nearly every capture a full 5ms sleep, twice. The reason is that the drain thread reaches EOF a few *microseconds* after `wait` returned — the child has only just exited — so the first `is_finished` check almost always loses the race and the second one is 5ms later. That is invisible in the tests and plain in a measurement: 60 sequential captures of `sh -c 'printf hello'` went 759us each before this branch and 4.54ms each on it, a 6x regression on a path `dl` takes dozens of times per launch. The drain thread now sets `ended` under the lock as it leaves and signals a condvar, and `collect` waits on that condvar with the same `DRAIN_GRACE` deadline. `ended` is tested under the same lock the wait releases, so no wakeup can be lost. The bound is unchanged; only the waiting is exact, and the `JoinHandle` goes with it — nothing ever joined it. Measured back to 678-785us per capture. The grandchild cases still return inside their bound: a timed-out capture in 201ms, a session in 201ms, and a clean exit whose grandchild holds both pipes in 1.0s (`DRAIN_GRACE` once per stream, the two collected in turn).
The drain's hand-rolled read loop replaced `read_to_end`, and with it lost the `ErrorKind::Interrupted` retry that came for free: `Ok(0) | Err(_) => break` treats a signal arriving mid-read as EOF. The harm is the worst shape available here. The bytes read so far are handed back inside an `Outcome::Ran` with a success exit, so a caller that parses this text — which is what `capture` is for — gets a prefix of git's answer and no indication anything went wrong. Silent truncation reported as success is exactly what keeping the bytes behind a lock was meant to avoid. Not reachable through `dl` today: the only handler the workspace installs is a glibc `signal()` (dl/src/lib.rs), which carries BSD `SA_RESTART` semantics, so nothing generates EINTR on a pipe read here. It was still a robustness regression against the code this replaced, and nothing but a test keeps it from becoming reachable the first time someone reaches for `sigaction` without `SA_RESTART`. Pinned by a reader that yields "ab", EINTR, "cd", EINTR, "ef": red before this it drained "ab", green after it drains "abcdef". A fake reader rather than a real pipe, deliberately — a signal landing mid-read is the one condition a real pipe will not produce on demand.
1 participant
A
capturethat timed out never returned when the killed child had left a descendant holding the inherited stdout/stderr pipe —read_to_endreturns only at pipe EOF, so the drain-thread join blocked forever andOutcome::TimedOutnever came back. Reachable in production: seven capture-with-timeout verbs indevlaunch-core/src/clients/{git,gh}.rs, andgit fetchover ssh forks exactly that shape (ControlMaster). Three captures pass no timeout at all —git clone --bare,git push -u, the launch-path fetch — so for those the hang is unbounded.What landed
Liveness never rests on a join. The drained bytes live behind a
Mutexrather than in the reading thread's return value, so they can be taken without joining it.collectwaits up toDRAIN_GRACE(500ms) for the pipe to reach EOF and then abandons the reader with the pipe it will never see the end of. The bound covers both paths that previously hung:setsid'd descendant still holds the pipe.session's stderr reader gets the same treatment: it is joined only when the pipe closed of its own accord, and abandoned on timeout.Captures stay in this process's group, and the kill stays single-pid. An earlier revision moved captures into a group of their own so the expiry kill could be a
killpgand take the tree down. That was reverted (4e78d8a), because a capture pipes stdout and stderr but not stdin, and/dev/ttyis reachable whatever stdin is: ssh's host-key confirmation, ssh's passphrase prompt and git's credential prompt all read the terminal from inside a capture. A child outside the terminal's foreground group takes SIGTTIN on that read and stops — andtry_waitnever reports a stopped child, so the wait runs to its deadline, or forever for the three captures that pass no timeout. Group membership is also the only thing that delivers a terminal Ctrl-C, sincecapture(unlikepassthrough) notes no foreground child for the interrupt handler tokillpg.So the tree a timed-out capture forked is knowingly left running. That is
main's existing behaviour, not something this PR introduces, and the thing worth guarding is guarded:tests/terminal.rsfails loudly if anyone reintroduces thekillpg.Tests
src/tests.rsgains three regression tests whose child forks asetsidgrandchild holding the pipe — the shape the old suite dodged, sinceexec sleep 30guarantees a single process — each bounded on a thread of its own so a red run fails instead of stalling the suite, and each guarded byrequire_setsid()so it cannot go green having pinned nothing.tests/terminal.rsis new: a real pty, because what a captured child may do with the terminal is a property of its process group and acargo testprocess has no controlling terminal to observe it from. It pins both halves of the reverted decision — that a captured child may read/dev/tty, and that a terminal Ctrl-C reaches it.Review follow-ups in this PR
perf: the drain's bounded wait is a condvar, not a 5ms poll— the bound was right, the waiting was not. PollingJoinHandle::is_finishedatPOLL_INTERVALcharged nearly every capture a 5ms sleep twice, because the drain thread reaches EOF microseconds after the wait for the child returned. Measured 759µs/capture before this branch, 4.54ms on it, 678–785µs after the fix.fix: an interrupted read is a retry, not the end of the pipe— the hand-rolled read loop had lostread_to_end's freeEINTRretry, so a signal mid-read read as EOF and truncated output was returned insideOutcome::Ranwith a success exit. Unreachable throughdltoday (glibcsignal()carriesSA_RESTART), but a robustness regression against the code it replaced.Verified:
cargo testper-suite across the workspace (~1,653 pass),cargo clippy --locked --all-targets -- -D warnings,cargo fmt --check,pytest(293 pass) all clean. The affected suites were repeated 75× including under 8 CPU hogs at--test-threads=8, with no flakes.Closes #302.
🤖 Generated with Claude Code