From 451f83b1d30f486059a18bd3d16d79a95f1362da Mon Sep 17 00:00:00 2001 From: Cameron Taggart Date: Sun, 16 Aug 2026 18:05:23 +0100 Subject: [PATCH] fix(core): avoid deadlock on here-documents larger than the pipe buffer `setup_open_file_with_contents` creates an anonymous pipe and writes the entire here-document / here-string payload into it before the reading command exists. That write can only complete while the payload still fits in the pipe's buffer, so any larger payload blocks forever. Linux papered over this by growing the buffer to the payload size with F_SETPIPE_SZ. Every other platform kept the default buffer -- 4 KiB on Windows -- so a here-document past that threshold hangs the shell. The Linux path had gaps of its own: the fcntl is propagated with `?`, so it also failed outright for payloads over /proc/sys/fs/pipe-max-size (1 MiB by default for unprivileged processes) and for a zero-length payload. Keep the resize as a best-effort fast path and hand oversized payloads to a helper thread instead, so the write proceeds while the reader drains. The thread ends when the payload is consumed, or earlier with a broken pipe if the read end is dropped by a command that never reads its stdin. Found running an 88k-line bash CLI under brush on Windows: every `cat < --- brush-core/src/interp.rs | 48 ++++++++++++++++++++---- brush-shell/tests/cases/compat/here.yaml | 26 +++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index ddf637f11..a39997ba2 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -1968,22 +1968,56 @@ fn setup_process_substitution( Ok((candidate_fd_num, target_file)) } +/// Largest payload we will write directly into a freshly created pipe without +/// a helper thread. +/// +/// Nothing is reading from the other end of the pipe yet, so an inline +/// `write_all` can only complete if the whole payload fits in the pipe's +/// buffer. 4 KiB is the smallest default buffer across the platforms we +/// target (it is the Windows default; Linux and macOS are larger), so staying +/// at or below it is safe everywhere. +const MAX_INLINE_PIPE_WRITE: usize = 4096; + +/// Creates a pipe whose read end is preloaded with `contents`, for use in +/// backing here-documents and here-strings. fn setup_open_file_with_contents(contents: &str) -> Result { let (reader, mut writer) = std::io::pipe()?; let bytes = contents.as_bytes(); + // On Linux we can try to grow the pipe's buffer to fit the entire payload, which + // lets us write it inline and hand back a pipe that's already fully populated. + // This is best-effort: the request fails for payloads beyond + // /proc/sys/fs/pipe-max-size (1 MiB by default for unprivileged processes) and for + // a zero-length payload. Any failure just means we fall back to the helper thread + // below, so we deliberately ignore the result instead of failing the redirection. #[cfg(any(target_os = "linux", target_os = "android"))] - { + let resized_to_fit = { use std::os::fd::AsFd as _; - let len = i32::try_from(bytes.len()) - .map_err(|_err| error::Error::from(error::ErrorKind::TooMuchData))?; - nix::fcntl::fcntl(reader.as_fd(), nix::fcntl::FcntlArg::F_SETPIPE_SZ(len))?; - } + i32::try_from(bytes.len()).is_ok_and(|len| { + nix::fcntl::fcntl(reader.as_fd(), nix::fcntl::FcntlArg::F_SETPIPE_SZ(len)).is_ok() + }) + }; + #[cfg(not(any(target_os = "linux", target_os = "android")))] + let resized_to_fit = false; - writer.write_all(bytes)?; - drop(writer); + if resized_to_fit || bytes.len() <= MAX_INLINE_PIPE_WRITE { + writer.write_all(bytes)?; + drop(writer); + } else { + // The payload is too large to be absorbed by the pipe's buffer, so writing it + // here would block forever: the reader is the command we haven't spawned yet. + // Hand the write off to a helper thread instead. It exits once the payload is + // consumed, or earlier with a broken-pipe error if the read end is dropped + // first (e.g. a command that never reads its stdin). + let contents = contents.to_owned(); + std::thread::Builder::new() + .name(String::from("brush-pipe-writer")) + .spawn(move || { + let _ = writer.write_all(contents.as_bytes()); + })?; + } Ok(reader.into()) } diff --git a/brush-shell/tests/cases/compat/here.yaml b/brush-shell/tests/cases/compat/here.yaml index 596c95477..fe6ecb8fe 100644 --- a/brush-shell/tests/cases/compat/here.yaml +++ b/brush-shell/tests/cases/compat/here.yaml @@ -281,3 +281,29 @@ cases: EOF ) echo $var + + - name: "Here doc larger than pipe buffer" + stdin: | + payload= + i=0 + while [ $i -lt 500 ]; do + payload="${payload}0123456789012345678901234567890123456789" + i=$((i + 1)) + done + out=$(cat <