From a6b348eb8ea98b837f74c233676fe60a93a80036 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Mon, 15 Dec 2025 05:32:52 +0000 Subject: [PATCH 1/2] tail: fix --pid with FIFO by using non-blocking open --- .../cspell.dictionaries/jargon.wordlist.txt | 1 + Cargo.lock | 1 + src/uu/tail/Cargo.toml | 3 ++ src/uu/tail/src/tail.rs | 47 ++++++++++++++++++- tests/by-util/test_tail.rs | 39 +++++++++++++++ 5 files changed, 90 insertions(+), 1 deletion(-) diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index a757953b44f..9426f2a1e3d 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -202,3 +202,4 @@ TUNABLES tunables VMULL vmull +SETFL diff --git a/Cargo.lock b/Cargo.lock index dae4e963cf5..89408f41532 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3940,6 +3940,7 @@ dependencies = [ "fluent", "libc", "memchr", + "nix", "notify", "rstest", "same-file", diff --git a/src/uu/tail/Cargo.toml b/src/uu/tail/Cargo.toml index 7d7b57a74a3..cfe400473c9 100644 --- a/src/uu/tail/Cargo.toml +++ b/src/uu/tail/Cargo.toml @@ -27,6 +27,9 @@ uucore = { workspace = true, features = ["fs", "parser-size"] } same-file = { workspace = true } fluent = { workspace = true } +[target.'cfg(unix)'.dependencies] +nix = { workspace = true, features = ["fs"] } + [target.'cfg(windows)'.dependencies] windows-sys = { workspace = true, features = [ "Win32_System_Threading", diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index cd10203b3f7..3bfa36a9d07 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -154,7 +154,9 @@ fn tail_file( } observer.add_bad_path(path, input.display_name.as_str(), false)?; } else { - match File::open(path) { + let open_result = open_file(path, settings.pid != 0); + + match open_result { Ok(mut file) => { let st = file.metadata()?; let blksize_limit = uucore::fs::sane_blksize::sane_blksize_from_metadata(&st); @@ -199,6 +201,49 @@ fn tail_file( Ok(()) } +/// Opens a file, using non-blocking mode for FIFOs when `use_nonblock_for_fifo` is true. +/// +/// When opening a FIFO with `--pid`, we need to use O_NONBLOCK so that: +/// 1. The open() call doesn't block waiting for a writer +/// 2. We can periodically check if the monitored process is still alive +/// +/// After opening, we clear O_NONBLOCK so subsequent reads block normally. +/// Without `--pid`, FIFOs block on open() until a writer connects (GNU behavior). +#[cfg(unix)] +fn open_file(path: &Path, use_nonblock_for_fifo: bool) -> std::io::Result { + use nix::fcntl::{FcntlArg, OFlag, fcntl}; + use std::fs::OpenOptions; + use std::os::fd::AsFd; + use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; + + let is_fifo = path + .metadata() + .ok() + .is_some_and(|m| m.file_type().is_fifo()); + + if is_fifo && use_nonblock_for_fifo { + let file = OpenOptions::new() + .read(true) + .custom_flags(libc::O_NONBLOCK) + .open(path)?; + + // Clear O_NONBLOCK so reads block normally + if let Ok(flags) = fcntl(file.as_fd(), FcntlArg::F_GETFL) { + let new_flags = OFlag::from_bits_truncate(flags) & !OFlag::O_NONBLOCK; + let _ = fcntl(file.as_fd(), FcntlArg::F_SETFL(new_flags)); + } + + Ok(file) + } else { + File::open(path) + } +} + +#[cfg(not(unix))] +fn open_file(path: &Path, _use_nonblock_for_fifo: bool) -> std::io::Result { + File::open(path) +} + fn tail_stdin( settings: &Settings, header_printer: &mut HeaderPrinter, diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 3f6455c21b2..a2efd05da07 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -2653,6 +2653,45 @@ fn test_fifo() { } } +/// Test that tail with --pid exits when the monitored process dies, even with a FIFO. +/// Without non-blocking FIFO open, tail would block forever waiting for a writer. +#[test] +#[cfg(all( + not(target_vendor = "apple"), + not(target_os = "windows"), + not(target_os = "android"), + not(target_os = "freebsd"), + not(target_os = "openbsd") +))] +fn test_fifo_with_pid() { + use std::process::Command; + + let (at, mut ucmd) = at_and_ucmd!(); + at.mkfifo("FIFO"); + + let mut dummy = Command::new("sh").spawn().unwrap(); + let pid = dummy.id(); + + let mut child = ucmd + .arg("-f") + .arg(format!("--pid={pid}")) + .arg("FIFO") + .run_no_wait(); + + child.make_assertion_with_delay(500).is_alive(); + + kill(Pid::from_raw(i32::try_from(pid).unwrap()), Signal::SIGUSR1).unwrap(); + let _ = dummy.wait(); + + child + .make_assertion_with_delay(DEFAULT_SLEEP_INTERVAL_MILLIS) + .is_not_alive() + .with_all_output() + .no_stderr() + .no_stdout() + .success(); +} + #[test] #[cfg(unix)] #[ignore = "disabled until fixed"] From 5e2039d17e1d66016163a17985f183ffe35d967f Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 10 Jan 2026 08:47:12 +0000 Subject: [PATCH 2/2] Address review comments: propagate fcntl errors and remove Windows stub --- src/uu/tail/src/tail.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index 86bdba653e2..a35e542d8a0 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -152,7 +152,10 @@ fn tail_file( } observer.add_bad_path(path, input.display_name.as_str(), false)?; } else { + #[cfg(unix)] let open_result = open_file(path, settings.pid != 0); + #[cfg(not(unix))] + let open_result = File::open(path); match open_result { Ok(mut file) => { @@ -226,10 +229,9 @@ fn open_file(path: &Path, use_nonblock_for_fifo: bool) -> std::io::Result .open(path)?; // Clear O_NONBLOCK so reads block normally - if let Ok(flags) = fcntl(file.as_fd(), FcntlArg::F_GETFL) { - let new_flags = OFlag::from_bits_truncate(flags) & !OFlag::O_NONBLOCK; - let _ = fcntl(file.as_fd(), FcntlArg::F_SETFL(new_flags)); - } + let flags = fcntl(file.as_fd(), FcntlArg::F_GETFL)?; + let new_flags = OFlag::from_bits_truncate(flags) & !OFlag::O_NONBLOCK; + fcntl(file.as_fd(), FcntlArg::F_SETFL(new_flags))?; Ok(file) } else { @@ -237,11 +239,6 @@ fn open_file(path: &Path, use_nonblock_for_fifo: bool) -> std::io::Result } } -#[cfg(not(unix))] -fn open_file(path: &Path, _use_nonblock_for_fifo: bool) -> std::io::Result { - File::open(path) -} - fn tail_stdin( settings: &Settings, header_printer: &mut HeaderPrinter,