fix: mapped tail builtin broken pipe to silent exit 141

- Handled broken pipe errors across tail output and follow paths by translating them to a silent SIGPIPE exit code.
- Prevented stderr noise when downstream pipeline readers exit early, matching native tail behavior.
This commit is contained in:
can1357
2026-08-03 18:53:14 +02:00
parent cdd1d13079
commit 451eb9842c
6 changed files with 92 additions and 32 deletions
+7 -6
View File
@@ -6622,21 +6622,22 @@ mod tests {
/// Regression: builtin tail upstream of an early-exiting consumer printed
/// "tail: Broken pipe" and failed (`tail -c N big.jsonl | jq …` with jq
/// aborting on a parse error). Real tail dies silently from SIGPIPE; the
/// builtin must exit 141 with no diagnostic, leaving the pipeline status
/// to the last stage.
/// builtin must exit 141 with no diagnostic. `pipefail` exposes tail's own
/// status, so rc=141 proves the broken pipe actually fired (head closed
/// the pipe early) and was mapped to the silent SIGPIPE exit, not 1.
#[tokio::test(flavor = "multi_thread")]
async fn tail_builtin_is_silent_when_downstream_closes_pipe() {
let dir = tempfile::tempdir().expect("tempdir");
let file = dir.path().join("big.txt");
// ~589 KiB: forces the seekable bounded_tail path and overflows the OS
// pipe buffer so tail is still writing when head exits.
// ~589 KiB: forces the seekable bounded_tail path, and the 400 KB tail
// overflows the OS pipe buffer so tail is still writing when head exits.
let command = format!(
"seq 1 100000 > '{file}'; tail -c 400000 '{file}' | head -c 10 > /dev/null; echo rc=$?",
"seq 1 100000 > '{file}'; set -o pipefail; tail -c 400000 '{file}' | head -c 10 > /dev/null; echo rc=$?",
file = file.display()
);
let (result, output) = execute_captured(command).await;
assert_eq!(result.exit_code, Some(0));
assert!(output.contains("rc=0"), "{output:?}");
assert!(output.contains("rc=141"), "{output:?}");
assert!(!output.contains("Broken pipe"), "{output:?}");
}
+11 -13
View File
@@ -15,11 +15,9 @@
use std::{
collections::VecDeque,
fs::File,
io::{BufRead, Read, Seek, SeekFrom, Write},
io::{self, BufRead, Read, Seek, SeekFrom, Write},
};
use uucore::error::UResult;
/// When reading files in reverse in `bounded_tail`, this is the size of each
/// block read at a time.
pub const BLOCK_SIZE: u64 = 1 << 16;
@@ -209,10 +207,10 @@ impl BytesChunk {
/// Fills `self.buffer` with maximal [`BUFFER_SIZE`] number of bytes,
/// draining the reader by that number of bytes. If EOF is reached (so 0
/// bytes are read), it returns [`UResult<None>`]; otherwise, it returns
/// [`UResult<Some(bytes)>`], where bytes is the number of bytes read from
/// bytes are read), it returns `Ok(None)`; otherwise, it returns
/// `Ok(Some(bytes))`, where bytes is the number of bytes read from
/// the source.
pub fn fill(&mut self, filehandle: &mut impl BufRead) -> UResult<Option<usize>> {
pub fn fill(&mut self, filehandle: &mut impl BufRead) -> io::Result<Option<usize>> {
let num_bytes = filehandle.read(&mut self.buffer)?;
self.bytes = num_bytes;
if num_bytes == 0 {
@@ -286,7 +284,7 @@ impl BytesChunkBuffer {
/// let mut chunks = BytesChunkBuffer::new(num_print);
/// chunks.fill(&mut reader).unwrap();
/// ```
pub fn fill(&mut self, reader: &mut impl BufRead) -> UResult<()> {
pub fn fill(&mut self, reader: &mut impl BufRead) -> io::Result<()> {
let mut chunk = Box::new(BytesChunk::new());
// fill chunks with all bytes from reader and reuse already instantiated chunks
@@ -323,7 +321,7 @@ impl BytesChunkBuffer {
Ok(())
}
pub fn print(&self, writer: &mut impl Write) -> UResult<()> {
pub fn print(&self, writer: &mut impl Write) -> io::Result<()> {
for chunk in &self.chunks {
writer.write_all(chunk.get_buffer())?;
}
@@ -459,7 +457,7 @@ impl LinesChunk {
/// the [`BytesChunk::fill`] function besides that this function also counts
/// and stores the number of lines encountered while reading from
/// the `filehandle`.
pub fn fill(&mut self, filehandle: &mut impl BufRead) -> UResult<Option<usize>> {
pub fn fill(&mut self, filehandle: &mut impl BufRead) -> io::Result<Option<usize>> {
match self.chunk.fill(filehandle)? {
None => {
self.lines = 0;
@@ -517,7 +515,7 @@ impl LinesChunk {
///
/// * `writer`: must implement [`Write`]
/// * `offset`: An offset in number of lines.
pub fn write_lines(&self, writer: &mut impl Write, offset: usize) -> UResult<()> {
pub fn write_lines(&self, writer: &mut impl Write, offset: usize) -> io::Result<()> {
self.write_bytes(writer, self.calculate_bytes_offset_from(offset))
}
@@ -528,7 +526,7 @@ impl LinesChunk {
///
/// * `writer`: must implement [`Write`]
/// * `offset`: An offset in number of bytes.
pub fn write_bytes(&self, writer: &mut impl Write, offset: usize) -> UResult<()> {
pub fn write_bytes(&self, writer: &mut impl Write, offset: usize) -> io::Result<()> {
writer.write_all(self.get_buffer_with(offset))?;
Ok(())
}
@@ -565,7 +563,7 @@ impl LinesChunkBuffer {
/// chunks. If there are no chunks, for example because the piped stdin
/// contained no lines, or `num_print = 0` then `iterator.next` will return
/// None.
pub fn fill(&mut self, reader: &mut impl BufRead) -> UResult<()> {
pub fn fill(&mut self, reader: &mut impl BufRead) -> io::Result<()> {
let mut chunk = Box::new(LinesChunk::new(self.delimiter));
while chunk.fill(reader)?.is_some() {
@@ -620,7 +618,7 @@ impl LinesChunkBuffer {
Ok(())
}
pub fn write(&self, mut writer: impl Write) -> UResult<()> {
pub fn write(&self, mut writer: impl Write) -> io::Result<()> {
for chunk in &self.chunks {
chunk.write_bytes(&mut writer, 0)?;
}
+4 -2
View File
@@ -154,9 +154,11 @@ impl FileHandling {
self.header_printer.print(display_name.as_str());
}
// pi-uutils: a closed downstream reader must end the follow loop
// silently (SIGPIPE semantics), not as a printed tail error.
let mut writer = BufWriter::new(pi_uutils_ctx::stdout().lock());
chunks.print(&mut writer)?;
writer.flush()?;
chunks.print(&mut writer).map_err(crate::map_output_error)?;
writer.flush().map_err(crate::map_output_error)?;
self.last.replace(path.to_owned());
self.update_metadata(path, None);
+61 -11
View File
@@ -123,6 +123,43 @@ fn rewrite_bsd_invocation(argv: &[OsString]) -> Option<Result<Vec<OsString>, Str
Some(Ok(tac_argv))
}
/// Exit status of a process killed by SIGPIPE (128 + 13).
const SIGPIPE_EXIT_CODE: i32 = 141;
/// pi-uutils: marker for a closed stdout pipe. Real tail is killed silently by
/// SIGPIPE when the downstream reader exits early (`tail file | head`); the
/// in-process builtin cannot die by signal, so output errors map to this
/// sentinel and [`run`] turns it into a bare exit 141 with no diagnostic,
/// mirroring the fd builtin's convention.
#[derive(Debug)]
struct BrokenPipeExit;
impl std::fmt::Display for BrokenPipeExit {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("Broken pipe")
}
}
impl std::error::Error for BrokenPipeExit {}
impl UError for BrokenPipeExit {
fn code(&self) -> i32 {
SIGPIPE_EXIT_CODE
}
}
/// Maps a stdout write error into the tail exit contract: `BrokenPipe`
/// becomes the silent [`BrokenPipeExit`] sentinel, everything else the
/// standard uucore io wrapper. Used by every path that copies data to the
/// context stdout, including follow mode.
pub(crate) fn map_output_error(err: io::Error) -> Box<dyn UError> {
if err.kind() == ErrorKind::BrokenPipe {
Box::new(BrokenPipeExit)
} else {
err.into()
}
}
/// In-process builtin entry point. Unlike upstream's `#[uucore::main] uumain`,
/// this renders clap help/usage/version to the context streams and never calls
/// `std::process::exit`, so it is safe inside the long-lived host shell
@@ -160,6 +197,11 @@ pub fn run(args: Vec<OsString>) -> i32 {
Ok(()) => pi_uutils_ctx::exit_code(),
Err(err) => {
let code = err.code();
if code == SIGPIPE_EXIT_CODE {
// Downstream closed the pipe; exit silently like SIGPIPE would
// kill a real tail (see `BrokenPipeExit`).
return SIGPIPE_EXIT_CODE;
}
let _ = writeln!(pi_uutils_ctx::stderr(), "tail: {err}");
if code == 0 { 1 } else { code }
},
@@ -299,11 +341,11 @@ fn tail_file(
&& file.is_seekable(if input.is_stdin() { offset } else { 0 })
&& (!st.is_file() || st.len() > blksize_limit)
{
bounded_tail(&mut file, settings)?;
bounded_tail(&mut file, settings).map_err(map_output_error)?;
reader = BufReader::new(file);
} else {
reader = BufReader::new(file);
unbounded_tail(&mut reader, settings)?;
unbounded_tail(&mut reader, settings).map_err(map_output_error)?;
}
if input.is_tailable() {
observer.add_path(
@@ -391,7 +433,7 @@ fn tail_stdin(
// streaming (pipe) path.
header_printer.print_input(input);
let mut reader = BufReader::new(pi_uutils_ctx::stdin());
unbounded_tail(&mut reader, settings)?;
unbounded_tail(&mut reader, settings).map_err(map_output_error)?;
Ok(())
}
@@ -531,7 +573,7 @@ fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) {
/// end of the file, and then read the file "backwards" in blocks of size
/// `BLOCK_SIZE` until we find the location of the first line/byte. This ends up
/// being a nice performance win for very large files.
fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> {
fn bounded_tail(file: &mut File, settings: &Settings) -> io::Result<()> {
debug_assert!(!settings.presume_input_pipe);
let mut limit = None;
@@ -568,7 +610,7 @@ fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> {
Ok(())
}
fn unbounded_tail<T: Read>(reader: &mut BufReader<T>, settings: &Settings) -> UResult<()> {
fn unbounded_tail<T: Read>(reader: &mut BufReader<T>, settings: &Settings) -> io::Result<()> {
let mut writer = BufWriter::new(pi_uutils_ctx::stdout().lock());
match &settings.mode {
FilterMode::Lines(Signum::Negative(count), sep) => {
@@ -636,8 +678,8 @@ fn unbounded_tail<T: Read>(reader: &mut BufReader<T>, settings: &Settings) -> UR
// pi-uutils: upstream emulates Unix SIGPIPE on Windows by calling
// `std::process::exit(13)` on a broken-pipe flush. That would kill the
// long-lived host shell process. An in-process builtin must never
// `process::exit`; let the broken pipe surface as a normal `io::Error` and
// propagate to the caller, matching every other pi-uutils builtin.
// `process::exit`; the broken pipe surfaces as a normal `io::Error`, which
// callers map to the silent SIGPIPE exit via `map_output_error`.
writer.flush()?;
Ok(())
}
@@ -834,12 +876,13 @@ mod tests {
file.flush().expect("flush");
drop(file);
let stderr_buf = Arc::new(Mutex::new(Vec::new()));
let io = pi_uutils_ctx::ScopeIo {
stdin: Box::new(io::empty()),
stdin_fd: None,
stdin_is_search_input: false,
stdout: Box::new(BrokenPipeWriter),
stderr: Box::new(io::sink()),
stderr: Box::new(SharedWriter { buf: stderr_buf.clone() }),
cwd: std::env::temp_dir(),
env: HashMap::new(),
cancel: Arc::new(AtomicBool::new(false)),
@@ -849,7 +892,10 @@ mod tests {
crate::run(vec![OsString::from("tail"), OsString::from(&path)])
});
assert_ne!(code, 0, "broken pipe must surface as a non-zero exit, not a panic");
// Real tail dies silently from SIGPIPE (exit 128+13); the in-process
// builtin must match: no "tail: Broken pipe" noise on stderr.
assert_eq!(code, 141, "broken pipe must map to the silent SIGPIPE exit");
assert_eq!(String::from_utf8(stderr_buf.lock().clone()).unwrap(), "");
}
#[test]
@@ -877,12 +923,13 @@ mod tests {
}
let input = b"1\n2\n3\n4\n5\n".to_vec();
let stderr_buf = Arc::new(Mutex::new(Vec::new()));
let io = pi_uutils_ctx::ScopeIo {
stdin: Box::new(Cursor::new(input)),
stdin_fd: None,
stdin_is_search_input: false,
stdout: Box::new(BrokenPipeWriter),
stderr: Box::new(io::sink()),
stderr: Box::new(SharedWriter { buf: stderr_buf.clone() }),
cwd: std::env::temp_dir(),
env: HashMap::new(),
cancel: Arc::new(AtomicBool::new(false)),
@@ -892,6 +939,9 @@ mod tests {
crate::run(vec![OsString::from("tail"), OsString::from("-n"), OsString::from("3")])
});
assert_ne!(code, 0, "broken pipe must surface as a non-zero exit, not process::exit");
// Same silent-SIGPIPE contract as the bounded test: exit 141, no
// diagnostic — this is the `tail -c N file | jq | …` regression.
assert_eq!(code, 141, "broken pipe must map to the silent SIGPIPE exit");
assert_eq!(String::from_utf8(stderr_buf.lock().clone()).unwrap(), "");
}
}
+5
View File
@@ -2,6 +2,11 @@
## [Unreleased]
### Fixed
- Fixed the built-in `tail` printing `tail: Broken pipe` and failing when a downstream pipeline reader exited early (e.g. `tail -c N file.jsonl | jq …` with jq aborting on a parse error); it now exits silently with 141 (128+SIGPIPE) like a real tail, in every output path including `--follow`.
- Fixed the in-process ps shell builtin rejecting common procps/BSD format specifiers (`ps -o tpgid,...` failed with `unknown output format specifier`); added `tpgid`, `pri`, `flags`, real/effective user and group columns, `wchan`, fault counters, `sz`, and the STAT `+` foreground flag.
## [17.2.6] - 2026-08-03
### Added
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Added
- Added the missing procps/BSD output format specifiers to the in-process ps shell builtin: `tpgid`, `pri`, `f`/`flags`, `ruser`/`logname`, `ruid`, `rgroup`, `rgid`, `group`/`egroup`, `gid`/`egid`, `wchan`, `min_flt`/`maj_flt`, `times`/`cputimes`, `sz`, single-character `s`/`state`, and aliases `tgid`/`tid`/`spid`, `euser`, `bsdtime`, and `rsz`. `ps -j` now includes a TPGID column, `ps -l` prints the single-character S column, and STAT gains the `+` foreground flag for processes in their terminal's foreground process group.
## [17.2.6] - 2026-08-03
### Added