fix(init): bound the whole tailscale query, not just the child's exit
Bound the whole tailscale query, not just the child's exit
Main's shard 2 is red on every PR, including ones that do not touch this code:
FAIL rust-test (2) — init::tests::a_tailscale_query_that_does_not_answer_is_no_suggestion
panicked at crates/preloop-cli/src/init.rs:4147:9:
an unresponsive daemon must not hang the wizard, took 30.024862264s
30.02s is the test's own fixture (sh -c "sleep 30"), so the child was awaited
to completion although bounded_stdout had been handed a 200 ms deadline.
Root cause
bounded_stdout watched the clock only while waiting for the child to exit. It
then blocked in reader.join() — that is, in read_to_end on the child's
stdout — until the pipe reached EOF. Child::kill reaches the direct child
alone, so a child that forked the command it was given leaves the pipe's write
end open in a grandchild that outlives the signal; the read then waits for the
grandchild and the deadline buys nothing.
Whether that happens is the host's /bin/sh:
- engine (
ubuntu:24.04runner,ci/runner.Dockerfile) →/bin/shis dash,
which forks-crather than replacing itself: the panic. Checked directly,
not inferred: the runner image isFROM ubuntu:24.04, and/bin/shin it is a
symlink to dash. - macOS →
/bin/shexecs the single command, so the pipe closes with the
child and the test passes locally.
Measured with the function lifted verbatim from main (a98e769e9, no workspace
build needed) in a rust:1.99-slim-bookworm container, where
/bin/sh -> dash (200 ms deadline, child sleeps 30 s):
| probe | main | this branch |
|---|---|---|
sh -c 'sleep 30' | 30.00983369s | 213.458463ms |
dash -c 'sleep 30' | 30.014565131s | 216.580719ms |
bash -c 'sleep 30' | 213.291085ms | 212.277528ms |
The healthy shapes are unchanged: echo hello → Some("hello") in 53.5ms,
exit 3 → None, a binary that is not on PATH → None, a 200 000-byte answer
→ Some(200000 bytes) in 50.6ms.
Fix
The drain hands its bytes back over a channel, and this thread waits for them
only for what is left of the deadline:
let (sender, answer) = std::sync::mpsc::channel();
std::thread::spawn(move || {
let _ = stdout.read_to_end(&mut bytes);
let _ = sender.send(bytes);
});
…
let remaining = deadline.saturating_duration_since(std::time::Instant::now());
let bytes = answer.recv_timeout(remaining).ok()?;
timeout now bounds the whole call. A child that answered has closed its own
stdout and sends at once, so nothing healthy waits longer than it used to. The
kill still happens and the child is still reaped — only the read stops waiting
for whatever the child left behind, which a double-forked descendant would
escape a process-group kill for anyway.
Reproduced locally, on the real test binary
fakebin/sh first on PATH forks the command instead of exec-ing it, exactly as
dash does, so a macOS host can hold the engine's condition:
| binary | condition | result |
|---|---|---|
| main | forking sh | FAIL — an unresponsive daemon must not hang the wizard, took 30.051644917s at init.rs:4147:9 |
| this branch | forking sh | pass — 0.262s wall, of which 0.050s is process start |
10 runs each with 8 CPU burners running (8 cores, one burner per core):
- forking
sh(engine condition): 10/10 pass —
0.228 0.228 0.228 0.226 0.228 0.230 0.237 0.229 0.255 0.235 s - host
sh: 10/10 pass —
0.287 0.311 0.235 0.237 0.248 0.242 0.238 0.239 0.245 0.248 s
The assertion is untouched: elapsed >= 200ms still holds (the deadline is
waited out, not short-circuited) and elapsed < 10s is met with two orders of
magnitude of room. No sleep, no widened threshold.
Wizard path audit
There is no unbounded tailscale call site to route. Both queries the wizard
makes go through tailscale_json → bounded_stdout (init.rs:2584-2588),
including the funnel status --json → serve status --json fallback, and
TAILSCALE_TIMEOUT is a 5 s const (init.rs:2549) — not an env default above
30 s. The unbounded wait was inside the helper the probe already used.
For completeness, the file's other process spawns: docker build/save/image inspect (init.rs:3210-3248, legitimately long — a base image build), the
same via status()/output() in run_container, and a macOS-only
sysctl -n kern.hv_support (init.rs:2918). None is on the tailscale probe.
cargo fmt on main
The first commit reformats init.rs:4259, which the pinned rustfmt
(rustfmt 1.10.0-stable (58cf8f9c35), the toolchain's, confirmed in the same
container) already rejects on main — cargo fmt --all --check cannot pass
without it. Formatting only, no behaviour.
The second commit is the fix plus a CHANGELOG.md entry under
[Unreleased] → Fixed.
Gates
cargo fmt --all --check— clean (and red on main without the first commit).cargo clippy --locked -p preloop-cli --all-targets -- -D warnings— clean,
Finished dev profile … in 14.54s, no warnings.- the failing test, 10× with the slow fixture and 8 burners — 20/20 pass
(10 with a forkingsh, 10 with the host's). cargo test -p preloop-cli init::tests— 31 passed, 0 failed.cargo test -p preloop-cli— all targets pass, 0 failed.
Summary by CodeRabbit
- Bug Fixes
- The wizard’s Tailscale query now respects a single five-second deadline while waiting for a response. It no longer waits indefinitely when a child process leaves the output stream open.
- Responses are limited to 4 MiB, preventing excessive output from growing without bound. If a complete response isn’t received before the deadline, the query returns no result.