From b78b74507a4a1fb1419c50167b3bba9667dca261 Mon Sep 17 00:00:00 2001 From: yor1xd Date: Thu, 28 May 2026 09:54:18 -0300 Subject: [PATCH 1/4] tee: do not retry on EINTR --- src/uu/tee/src/tee.rs | 33 ++++++++++++++++++--------- tests/by-util/test_tee.rs | 47 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index 8f96f4cc73..1cc8bc06d9 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -249,18 +249,31 @@ enum Writer { impl Writer { pub fn write_all(&mut self, buf: &[u8]) -> Result<()> { - match self { - // File does not have line buffering - Self::File(f) => f.write_all(buf), - #[cfg(any(unix, target_os = "wasi"))] - Self::Stdout(s) => s.write_all(buf), - #[cfg(not(any(unix, target_os = "wasi")))] - Self::Stdout(s) => { - s.write_all(buf)?; - // needs unsafe to remove buffering... flush after write_all to keep overhead minimal - s.flush() + let mut buf = buf; + let writer: &mut dyn Write = match self { + Self::File(f) => f, + Self::Stdout(s) => s, + }; + while !buf.is_empty() { + match writer.write(buf) { + Ok(0) => { + return Err(Error::new( + ErrorKind::WriteZero, + "failed to write whole buffer", + )); + } + Ok(n) => { + buf = &buf[n..]; + #[cfg(not(any(unix, target_os = "wasi")))] + if let Self::Stdout(s) = self { + // needs unsafe to remove buffering... flush after write_all to keep overhead minimal + writer.flush()?; + } + } + Err(e) => return Err(e), } } + Ok(()) } } diff --git a/tests/by-util/test_tee.rs b/tests/by-util/test_tee.rs index 2948eaa045..50429cfc43 100644 --- a/tests/by-util/test_tee.rs +++ b/tests/by-util/test_tee.rs @@ -697,6 +697,53 @@ mod linux_only { assert_eq!(at.read(file_out_b), content); assert!(result.stderr_str().contains("No space left on device")); } + + #[test] + fn test_eintr_on_write_is_not_retried() { + if std::process::Command::new("strace") + .arg("--version") + .output() + .is_err() + { + return; + } + + use std::io::Write; + + let mut child = std::process::Command::new("strace") + .args([ + "-o", + "/dev/null", // suppress strace's own output + "-e", + "inject=write:error=EINTR:when=1", + env!("CARGO_BIN_EXE_coreutils"), + "tee", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("failed to spawn strace"); + + // Write something so tee actually calls write() + if let Some(mut stdin) = child.stdin.take() { + let _ = stdin.write_all(b"hello\n"); + // drop stdin to send EOF + } + + let result = child.wait_with_output().expect("failed to wait"); + + let stderr = String::from_utf8_lossy(&result.stderr); + + assert!( + !result.status.success(), + "tee should have failed on EINTR but exited zero.\nstderr: {stderr}" + ); + assert!( + stderr.contains("Interrupted system call"), + "expected 'Interrupted system call' in stderr, got: {stderr}" + ); + } } // Additional cross-platform tee tests to cover GNU compatibility around --output-error From 4166ea0bf7352df466b96e7e57c4551a0a0ed24b Mon Sep 17 00:00:00 2001 From: yor1xd Date: Thu, 28 May 2026 10:25:11 -0300 Subject: [PATCH 2/4] tee: fix flush on non-unix stdout and refactor write_all --- src/uu/tee/src/tee.rs | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index 1cc8bc06d9..73e43aab18 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -250,27 +250,22 @@ enum Writer { impl Writer { pub fn write_all(&mut self, buf: &[u8]) -> Result<()> { let mut buf = buf; - let writer: &mut dyn Write = match self { - Self::File(f) => f, - Self::Stdout(s) => s, - }; while !buf.is_empty() { - match writer.write(buf) { - Ok(0) => { - return Err(Error::new( - ErrorKind::WriteZero, - "failed to write whole buffer", - )); - } - Ok(n) => { - buf = &buf[n..]; - #[cfg(not(any(unix, target_os = "wasi")))] - if let Self::Stdout(s) = self { - // needs unsafe to remove buffering... flush after write_all to keep overhead minimal - writer.flush()?; + match self { + Self::File(f) => match f.write(buf) { + Ok(0) => return Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer")), + Ok(n) => buf = &buf[n..], + Err(e) => return Err(e), + }, + Self::Stdout(s) => match s.write(buf) { + Ok(0) => return Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer")), + Ok(n) => { + buf = &buf[n..]; + #[cfg(not(any(unix, target_os = "wasi")))] + s.flush()?; } - } - Err(e) => return Err(e), + Err(e) => return Err(e), + }, } } Ok(()) From 53761c00231a4b3563be221771e9d5aae82266de Mon Sep 17 00:00:00 2001 From: oech3 <79379754+oech3@users.noreply.github.com> Date: Thu, 28 May 2026 22:40:22 +0900 Subject: [PATCH 3/4] tee: catch EINTR at write --- src/uucore/src/lib/mods/io.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/uucore/src/lib/mods/io.rs b/src/uucore/src/lib/mods/io.rs index eccbd10e37..9c3c7ce7b0 100644 --- a/src/uucore/src/lib/mods/io.rs +++ b/src/uucore/src/lib/mods/io.rs @@ -53,6 +53,31 @@ impl io::Write for RawWriter { } } +// write_all retries with EINTR which is sometimes not compatible with GNU +#[cfg(any(unix, target_os = "wasi"))] +pub trait AsFdExt { + fn write_all_no_retry(&self, buf: &[u8]) -> io::Result<()>; +} +#[cfg(any(unix, target_os = "wasi"))] +impl AsFdExt for T { + fn write_all_no_retry(&self, mut buf: &[u8]) -> io::Result<()> { + let fd = self.as_fd(); + while !buf.is_empty() { + match rustix::io::write(fd, buf) { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "failed to write whole buffer", + )); + } + Ok(n) => buf = &buf[n..], + Err(e) => return Err(e.into()), + } + } + Ok(()) + } +} + /// abstraction wrapper for native file handle / file descriptor // todo: remove clone introducing additional syscall dependency pub struct OwnedFileDescriptorOrHandle { From 85a6062bdacf4cd17de599ef1c219c5591b3494b Mon Sep 17 00:00:00 2001 From: yor1xd Date: Thu, 28 May 2026 09:54:18 -0300 Subject: [PATCH 4/4] tee: do not retry on EINTR --- src/uu/tee/src/tee.rs | 33 +++++++++++++++------------------ tests/by-util/test_tee.rs | 3 +-- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index 73e43aab18..a3b9e4ea5d 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -249,26 +249,23 @@ enum Writer { impl Writer { pub fn write_all(&mut self, buf: &[u8]) -> Result<()> { - let mut buf = buf; - while !buf.is_empty() { - match self { - Self::File(f) => match f.write(buf) { - Ok(0) => return Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer")), - Ok(n) => buf = &buf[n..], - Err(e) => return Err(e), - }, - Self::Stdout(s) => match s.write(buf) { - Ok(0) => return Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer")), - Ok(n) => { - buf = &buf[n..]; - #[cfg(not(any(unix, target_os = "wasi")))] - s.flush()?; - } - Err(e) => return Err(e), - }, + #[cfg(any(unix, target_os = "wasi"))] + use uucore::io::AsFdExt as _; + match self { + // File does not have line buffering + #[cfg(any(unix, target_os = "wasi"))] + Self::File(f) => f.write_all_no_retry(buf), + #[cfg(not(any(unix, target_os = "wasi")))] + Self::File(f) => f.write_all(buf), + #[cfg(any(unix, target_os = "wasi"))] + Self::Stdout(s) => s.0.write_all_no_retry(buf), + #[cfg(not(any(unix, target_os = "wasi")))] + Self::Stdout(s) => { + s.write_all(buf)?; + // needs unsafe to remove buffering... flush after write_all to keep overhead minimal + s.flush() } } - Ok(()) } } diff --git a/tests/by-util/test_tee.rs b/tests/by-util/test_tee.rs index 50429cfc43..f6699875bd 100644 --- a/tests/by-util/test_tee.rs +++ b/tests/by-util/test_tee.rs @@ -700,6 +700,7 @@ mod linux_only { #[test] fn test_eintr_on_write_is_not_retried() { + use std::io::Write; if std::process::Command::new("strace") .arg("--version") .output() @@ -708,8 +709,6 @@ mod linux_only { return; } - use std::io::Write; - let mut child = std::process::Command::new("strace") .args([ "-o",