From 5037c42229b2c73cc9db0d6c5a1b72f3af5bd624 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Fri, 10 Apr 2026 23:24:53 -0700 Subject: [PATCH] tee: don't treat a short read as end-of-file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 13fb3bede ("tee: increase buf size for large input") split copy() into a one-shot first read followed by a loop over a larger buffer. The first branch returned early if `read(2)` returned fewer bytes than FIRST_BUF_SIZE: if bytes_count < FIRST_BUF_SIZE { output.flush()?; return Ok(len); } A short read is not EOF. POSIX `read(2)` only returns 0 at end-of-file; a return of N < buflen just means "that's what was available without blocking". Any pipeline where the upstream writer pauses between writes — a slow producer, a `sleep` in a shell pipeline, a systemd service emitting log lines in bursts — will see a short read on the very first call and will be cut off after one chunk. Downstream this manifests as the writer dying with SIGPIPE (exit 141) on its next write, because tee has already closed its end of the pipe. We hit this with a Go program writing structured logs through `2>&1 | tee -a /var/log/foo.log` in a systemd oneshot: the first slog line made it through, tee exited, and the next write killed the Go process before it could do any work. Keep the two-buffer optimization — it's useful for large inputs — but gate the upgrade to the larger buffer on actually seeing a full-sized read, and keep looping on the small buffer until we do. Only `read == 0` terminates the loop. Add a regression test that writes two chunks separated by a delay, drains the first one from tee's stdout so we know tee has processed its first read/write cycle, then writes the second chunk and asserts it makes it through to both stdout and the output file. Without this fix the second `write_in` fails with "Broken pipe (os error 32)". --- src/uu/tee/src/tee.rs | 29 +++++++++++++------- tests/by-util/test_tee.rs | 58 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index 272083b5c41..638f874268e 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -127,23 +127,32 @@ fn copy(mut input: impl Read, mut output: impl Write) -> Result { }; let mut buffer = [0u8; FIRST_BUF_SIZE]; let mut len = 0; - match input.read(&mut buffer) { - Ok(0) => return Ok(0), - Ok(bytes_count) => { - output.write_all(&buffer[0..bytes_count])?; - len = bytes_count; - if bytes_count < FIRST_BUF_SIZE { + // Read with the stack buffer until we observe a full-sized read, which + // is a hint that the input is big enough to benefit from a larger + // buffer. A *short* read does not imply EOF: `read(2)` only returns 0 + // at end-of-file, and a pipeline whose writer pauses between writes + // will commonly return less than the buffer size per call. We must + // therefore keep looping until we either see a full-sized read (and + // upgrade to the larger buffer) or `read` returns 0. + loop { + match input.read(&mut buffer) { + Ok(0) => return Ok(len), // end of file + Ok(bytes_count) => { + output.write_all(&buffer[..bytes_count])?; // flush the buffer to comply with POSIX requirement that // `tee` does not buffer the input. output.flush()?; - return Ok(len); + len += bytes_count; + if bytes_count == FIRST_BUF_SIZE { + break; + } } + Err(e) if e.kind() == ErrorKind::Interrupted => {} + Err(e) => return Err(e), } - Err(e) if e.kind() == ErrorKind::Interrupted => (), - Err(e) => return Err(e), } - // but optimize buffer size also for large file + // Optimize buffer size for large files. let mut buffer = vec![0u8; 4 * FIRST_BUF_SIZE]; //stack array makes code path for smaller file slower loop { match input.read(&mut buffer) { diff --git a/tests/by-util/test_tee.rs b/tests/by-util/test_tee.rs index ae3e34dcd14..2e0f54e5ecf 100644 --- a/tests/by-util/test_tee.rs +++ b/tests/by-util/test_tee.rs @@ -191,6 +191,64 @@ fn test_tee_output_not_buffered() { handle.join().unwrap(); } +#[test] +fn test_tee_continues_after_short_read() { + // Regression test: `tee` must keep reading until EOF even when the + // first `read(2)` returns fewer bytes than its internal buffer. This + // happens in any pipeline where the upstream writer pauses between + // writes (e.g. a slow producer, a `sleep` in a shell pipeline, or a + // service emitting log lines in bursts). Treating a short read as + // end-of-file caused tee to exit prematurely and downstream producers + // to die with SIGPIPE on their next write. + // + // Run in a separate thread so that the test fails via timeout rather + // than hanging if a regression reintroduces the bug in a form where + // tee blocks on read instead of exiting early. + let handle = std::thread::spawn(move || { + let (at, mut ucmd) = at_and_ucmd!(); + let file_out = "tee_short_read_out"; + + let mut child = ucmd + .arg(file_out) + .set_stdin(Stdio::piped()) + .set_stdout(Stdio::piped()) + .run_no_wait(); + + // First chunk — deliberately much smaller than tee's internal + // buffer so that `read(2)` returns a short count. + child.write_in(b"first\n"); + assert_eq!(&child.stdout_exact_bytes(6), b"first\n"); + + // Give a buggy implementation time to exit before we try to + // write again. + child.delay(50); + + // Second chunk. A correctly-implemented tee is still reading + // from stdin; a buggy one has already exited and this write + // will either fail with EPIPE or never reach the output file. + child.write_in(b"second\n"); + assert_eq!(&child.stdout_exact_bytes(7), b"second\n"); + + // `wait` closes stdin for us before waiting on the child. + child.wait().unwrap().success(); + + assert_eq!(at.read(file_out), "first\nsecond\n"); + }); + + for _ in 0..500 { + std::thread::sleep(Duration::from_millis(10)); + if handle.is_finished() { + break; + } + } + + assert!( + handle.is_finished(), + "tee did not complete within the timeout" + ); + handle.join().unwrap(); +} + #[cfg(target_os = "linux")] mod linux_only { use uutests::util::{AtPath, CmdResult, UCommand};