Skip to content

Commit 3441de9

Browse files
tail: fix --pid with FIFO by using non-blocking open (#9663)
* tail: fix --pid with FIFO by using non-blocking open * Address review comments: propagate fcntl errors and remove Windows stub --------- Co-authored-by: Sylvestre Ledru <[email protected]> Co-authored-by: Sylvestre Ledru <[email protected]>
1 parent bb88fb2 commit 3441de9

File tree

5 files changed

+87
-1
lines changed

5 files changed

+87
-1
lines changed

.vscode/cspell.dictionaries/jargon.wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,5 @@ TUNABLES
217217
tunables
218218
VMULL
219219
vmull
220+
SETFL
220221
tmpfs

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/uu/tail/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ uucore = { workspace = true, features = ["fs", "parser-size", "signals"] }
2727
same-file = { workspace = true }
2828
fluent = { workspace = true }
2929

30+
[target.'cfg(unix)'.dependencies]
31+
nix = { workspace = true, features = ["fs"] }
32+
3033
[target.'cfg(windows)'.dependencies]
3134
windows-sys = { workspace = true, features = [
3235
"Win32_System_Threading",

src/uu/tail/src/tail.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,12 @@ fn tail_file(
152152
}
153153
observer.add_bad_path(path, input.display_name.as_str(), false)?;
154154
} else {
155-
match File::open(path) {
155+
#[cfg(unix)]
156+
let open_result = open_file(path, settings.pid != 0);
157+
#[cfg(not(unix))]
158+
let open_result = File::open(path);
159+
160+
match open_result {
156161
Ok(mut file) => {
157162
let st = file.metadata()?;
158163
let blksize_limit = uucore::fs::sane_blksize::sane_blksize_from_metadata(&st);
@@ -197,6 +202,43 @@ fn tail_file(
197202
Ok(())
198203
}
199204

205+
/// Opens a file, using non-blocking mode for FIFOs when `use_nonblock_for_fifo` is true.
206+
///
207+
/// When opening a FIFO with `--pid`, we need to use O_NONBLOCK so that:
208+
/// 1. The open() call doesn't block waiting for a writer
209+
/// 2. We can periodically check if the monitored process is still alive
210+
///
211+
/// After opening, we clear O_NONBLOCK so subsequent reads block normally.
212+
/// Without `--pid`, FIFOs block on open() until a writer connects (GNU behavior).
213+
#[cfg(unix)]
214+
fn open_file(path: &Path, use_nonblock_for_fifo: bool) -> std::io::Result<File> {
215+
use nix::fcntl::{FcntlArg, OFlag, fcntl};
216+
use std::fs::OpenOptions;
217+
use std::os::fd::AsFd;
218+
use std::os::unix::fs::{FileTypeExt, OpenOptionsExt};
219+
220+
let is_fifo = path
221+
.metadata()
222+
.ok()
223+
.is_some_and(|m| m.file_type().is_fifo());
224+
225+
if is_fifo && use_nonblock_for_fifo {
226+
let file = OpenOptions::new()
227+
.read(true)
228+
.custom_flags(libc::O_NONBLOCK)
229+
.open(path)?;
230+
231+
// Clear O_NONBLOCK so reads block normally
232+
let flags = fcntl(file.as_fd(), FcntlArg::F_GETFL)?;
233+
let new_flags = OFlag::from_bits_truncate(flags) & !OFlag::O_NONBLOCK;
234+
fcntl(file.as_fd(), FcntlArg::F_SETFL(new_flags))?;
235+
236+
Ok(file)
237+
} else {
238+
File::open(path)
239+
}
240+
}
241+
200242
fn tail_stdin(
201243
settings: &Settings,
202244
header_printer: &mut HeaderPrinter,

tests/by-util/test_tail.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,6 +2659,45 @@ fn test_fifo() {
26592659
}
26602660
}
26612661

2662+
/// Test that tail with --pid exits when the monitored process dies, even with a FIFO.
2663+
/// Without non-blocking FIFO open, tail would block forever waiting for a writer.
2664+
#[test]
2665+
#[cfg(all(
2666+
not(target_vendor = "apple"),
2667+
not(target_os = "windows"),
2668+
not(target_os = "android"),
2669+
not(target_os = "freebsd"),
2670+
not(target_os = "openbsd")
2671+
))]
2672+
fn test_fifo_with_pid() {
2673+
use std::process::Command;
2674+
2675+
let (at, mut ucmd) = at_and_ucmd!();
2676+
at.mkfifo("FIFO");
2677+
2678+
let mut dummy = Command::new("sh").spawn().unwrap();
2679+
let pid = dummy.id();
2680+
2681+
let mut child = ucmd
2682+
.arg("-f")
2683+
.arg(format!("--pid={pid}"))
2684+
.arg("FIFO")
2685+
.run_no_wait();
2686+
2687+
child.make_assertion_with_delay(500).is_alive();
2688+
2689+
kill(Pid::from_raw(i32::try_from(pid).unwrap()), Signal::SIGUSR1).unwrap();
2690+
let _ = dummy.wait();
2691+
2692+
child
2693+
.make_assertion_with_delay(DEFAULT_SLEEP_INTERVAL_MILLIS)
2694+
.is_not_alive()
2695+
.with_all_output()
2696+
.no_stderr()
2697+
.no_stdout()
2698+
.success();
2699+
}
2700+
26622701
#[test]
26632702
#[cfg(unix)]
26642703
#[ignore = "disabled until fixed"]

0 commit comments

Comments
 (0)