Skip to main content

max / alloy

cli: write a streamed command's stdin off the render thread spawn_streaming wrote the secret to the child's stdin before returning, which is before run::spawn starts the threads that drain stdout. A child that writes before it reads fills its stdout pipe and stops there, so the write blocks against a child that will never read, and the reader that would unblock it is waiting on this function to return. A stage running `seq 1 20000; cat` with 256 KiB of stdin hangs the sequence outright. It has been safe only because the one secret in the installer is a `user:hash` line, about a hundred bytes against a 64 KiB pipe buffer. Nothing in the type states that bound or checks it. Write on a thread instead, and copy into another Secret so the thread's buffer is scrubbed on drop like the original.
Author: Max Johnson <me@maxj.phd> · 2026-07-22 21:53 UTC
Signed with PGP, not checked
Commit: 82e348eb6a268dd9f96667fccbb0a77f29486ccf
Parent: e289049
2 files changed, +50 insertions, -1 deletion
M crates/alloy/src/cli.rs +18 -1
@@ -335,12 +335,29 @@
335 335
336 336 // Same contract as `spawn`: the handle is dropped once written, so a
337 337 // child reading to EOF is not left waiting on a pipe nobody will close.
338 + //
339 + // On a thread, unlike `spawn`, because this returns to a caller that
340 + // has not started draining stdout yet. A child that writes before it
341 + // reads fills its stdout pipe and stops; writing stdin here then blocks
342 + // against a child that will never read, with the reader that would
343 + // unblock it waiting on this function to return. Today's only secret is
344 + // a `user:hash` line against a 64 KiB pipe buffer, so the write always
345 + // completes in one go and the deadlock stays theoretical. It is
346 + // theoretical because of a size nothing here states or checks.
347 + //
348 + // The copy is a `Secret` too, so the thread's buffer is scrubbed on
349 + // drop like the original. A failed write is not reported: the caller is
350 + // long gone by then, and a child denied its input fails on its own and
351 + // says why on stderr, which the sequence already streams.
338 352 if let Some(secret) = &self.stdin {
339 353 let mut pipe = child
340 354 .stdin
341 355 .take()
342 356 .context("stdin was piped but no handle came back")?;
343 - pipe.write_all(secret.expose())?;
357 + let secret = Secret::new(secret.expose().to_vec());
358 + std::thread::spawn(move || {
359 + let _ = pipe.write_all(secret.expose());
360 + });
344 361 }
345 362
346 363 Ok(child)
M crates/alloy/src/run.rs +32
@@ -709,6 +709,38 @@
709 709 assert_eq!(sequence.output(), ["Module is unknown"]);
710 710 }
711 711
712 + // Writing a command's stdin used to happen on the render thread, before
713 + // the pump threads existed. A child that writes before it reads fills its
714 + // stdout pipe and stops, so the write blocked against a child that would
715 + // never read, and the readers that would have unblocked it had not been
716 + // started yet. Safe today only because the one secret is about a hundred
717 + // bytes against a 64 KiB pipe buffer, which is a bound nothing states.
718 + //
719 + // Driven on its own thread because the failure is a deadlock: without this
720 + // the test would hang the suite rather than fail it.
721 + #[test]
722 + fn a_stdin_larger_than_the_pipe_buffer_does_not_deadlock() {
723 + // Both sides past 64 KiB, so neither fits in a buffer.
724 + let stdin = "x".repeat(256 * 1024);
725 + let (done, finished) = channel();
726 +
727 + thread::spawn(move || {
728 + let mut log = CommandLog::new();
729 + let mut sequence = Sequence::new(vec![Stage::Run(
730 + Invocation::new("sh")
731 + .args(["-c", "seq 1 20000; cat >/dev/null; echo read-it-all"])
732 + .stdin(Secret::new(stdin)),
733 + )]);
734 + drive(&mut sequence, &mut log);
735 + let _ = done.send(sequence.output().last().cloned());
736 + });
737 +
738 + match finished.recv_timeout(Duration::from_secs(30)) {
739 + Ok(last) => assert_eq!(last.as_deref(), Some("read-it-all")),
740 + Err(_) => panic!("writing stdin deadlocked against the child's stdout"),
741 + }
742 + }
743 +
712 744 // A resolver that cannot make sense of the output fails the sequence with
713 745 // its own message, rather than letting a later command fail obscurely.
714 746 #[test]