diff --git a/Cargo.lock b/Cargo.lock index f6c59ca9a87..47158db5bcf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3715,6 +3715,7 @@ dependencies = [ "clap", "crossterm", "fluent", + "rustix", "tempfile", "uucore", ] diff --git a/src/uu/more/Cargo.toml b/src/uu/more/Cargo.toml index 445a78e63ae..a1453a261b5 100644 --- a/src/uu/more/Cargo.toml +++ b/src/uu/more/Cargo.toml @@ -17,16 +17,18 @@ doctest = false [dependencies] clap = { workspace = true } -crossterm = { workspace = true, features = ["events"] } +# use-dev-tty: avoid mio's tight loop on tty EOF/hangup (see #15258); also +# needed on macOS for event init when stdin is a pipe. +crossterm = { workspace = true, features = ["events", "use-dev-tty"] } fluent = { workspace = true } uucore = { workspace = true } +[target.'cfg(unix)'.dependencies] +rustix = { workspace = true, features = ["event"] } + [target.'cfg(windows)'.dependencies] crossterm = { workspace = true, features = ["windows"] } -[target.'cfg(target_vendor = "apple")'.dependencies] -crossterm = { workspace = true, features = ["use-dev-tty"] } - [[bin]] name = "more" path = "src/main.rs" diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index ca9a6045120..d00bf4aacc4 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -389,6 +389,92 @@ fn reset_term() -> UResult<()> { Ok(()) } +/// True when the tty crossterm reads keys from has hung up, so we should stop +/// waiting for keys instead of spinning on EOF. +/// +/// Matches crossterm's `use-dev-tty` `tty_fd()` choice: stdin when it is a +/// terminal, otherwise `/dev/tty`. Never polls a non-tty stdin (e.g. `cat | +/// more`), which would report hangup as soon as the writer closes. +#[cfg(unix)] +fn keyboard_input_lost() -> bool { + use std::fs::OpenOptions; + use std::os::fd::{AsFd, BorrowedFd}; + use std::sync::OnceLock; + + fn fd_hung_up(fd: impl AsFd) -> bool { + // Request POLLIN: with `events=0`, Linux ppoll often times out without + // filling HUP/ERR into revents. POLLIN alone is ignored below. + let mut fds = [rustix::event::PollFd::new( + &fd, + rustix::event::PollFlags::IN, + )]; + match rustix::event::poll(&mut fds, Some(&rustix::event::Timespec::default())) { + Ok(_) => fds[0].revents().intersects( + rustix::event::PollFlags::HUP + | rustix::event::PollFlags::ERR + | rustix::event::PollFlags::NVAL, + ), + Err(_) => false, + } + } + + // Crossterm picks the key fd once (stdin if it is a tty, else `/dev/tty`). + // After hangup, `isatty(stdin)` becomes false — do not switch sources. + static KEY_FD_IS_STDIN: OnceLock = OnceLock::new(); + let key_fd_is_stdin = *KEY_FD_IS_STDIN.get_or_init(|| stdin().is_terminal()); + + if key_fd_is_stdin { + // SAFETY: stdin (fd 0) is open for the lifetime of the process. + fd_hung_up(unsafe { BorrowedFd::borrow_raw(0) }) + } else { + OpenOptions::new() + .read(true) + .write(true) + .open("/dev/tty") + .is_ok_and(fd_hung_up) + } +} + +#[cfg(not(unix))] +fn keyboard_input_lost() -> bool { + false +} + +/// Result of a non-blocking key wait. +enum KeyWait { + Idle, + Event(Event), + /// Key tty hung up; leave the pager cleanly. + Hangup, +} + +/// Wait for a key event without sitting inside crossterm's timed `poll`, which +/// busy-loops for the whole timeout once the key tty has hung up. +fn poll_key_event() -> UResult { + if keyboard_input_lost() { + return Ok(KeyWait::Hangup); + } + // Zero-timeout: one crossterm poll, then we re-check hangup ourselves. + match event::poll(Duration::ZERO) { + Ok(false) => { + std::thread::sleep(Duration::from_millis(50)); + Ok(KeyWait::Idle) + } + Ok(true) => { + if keyboard_input_lost() { + return Ok(KeyWait::Hangup); + } + match event::read() { + Ok(ev) => Ok(KeyWait::Event(ev)), + Err(_) if keyboard_input_lost() => Ok(KeyWait::Hangup), + Err(e) => Err(e.into()), + } + } + Err(_) if keyboard_input_lost() => Ok(KeyWait::Hangup), + Err(e) => Err(e.into()), + } +} + #[cfg(target_os = "fuchsia")] #[inline(always)] fn reset_term() -> UResult<()> { @@ -411,6 +497,8 @@ fn more( next_file: Option<&str>, options: &mut Options, ) -> UResult<()> { + // Cache which fd crossterm will use for keys before any hangup can flip isatty. + let _ = keyboard_input_lost(); // Initialize output let out = setup_term()?; // Ensure raw mode is disabled on drop @@ -552,15 +640,15 @@ impl<'a> Pager<'a> { return Ok(()); } loop { - if event::poll(Duration::from_millis(100))? - && let Event::Key(KeyEvent { + match poll_key_event()? { + KeyWait::Hangup + | KeyWait::Event(Event::Key(KeyEvent { code: KeyCode::Enter, modifiers: KeyModifiers::NONE, kind: KeyEventKind::Press, .. - }) = event::read()? - { - return Ok(()); + })) => return Ok(()), + KeyWait::Idle | KeyWait::Event(_) => {} } } } @@ -641,11 +729,13 @@ impl<'a> Pager<'a> { /// Process user input events until exit fn process_events(&mut self, options: &Options) -> UResult<()> { loop { - if !event::poll(Duration::from_millis(100))? { - continue; - } + let ev = match poll_key_event()? { + KeyWait::Hangup => return Ok(()), + KeyWait::Idle => continue, + KeyWait::Event(ev) => ev, + }; let mut wrong_key = None; - match event::read()? { + match ev { // --- Quit commands --- Event::Key( KeyEvent { diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 1fc51d068dd..efe18c1a9f1 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -260,6 +260,78 @@ fn test_from_line_option() { quit_more(&mut controller, child); } +/// When the key tty hangs up, `more` must exit instead of busy-looping on EOF. +#[test] +#[cfg(unix)] +fn test_exits_on_tty_hangup() { + let (path, controller, _replica) = pty_path(); + let (at, mut ucmd) = at_and_ucmd!(); + let content: String = (0..80).map(|i| format!("line {i}\n")).collect(); + at.write("hangup.txt", &content); + + // stdin is a tty, so crossterm (use-dev-tty) reads keys from stdin — the PTY. + let mut child = ucmd + .set_stdin(File::open(&path).unwrap()) + .set_stdout(File::create(&path).unwrap()) + .args(&["-n", "5", "hangup.txt"]) + .run_no_wait(); + + child.delay(300); + drop(controller); // hang up the key tty + + for _ in 0..50 { + if !child.is_alive() { + return; + } + child.delay(100); + } + child.kill(); + panic!("more still running after key tty hangup"); +} + +/// Piped content stdin must not be treated as the key tty: closing the writer +/// (POLLHUP on the data pipe) must not make `more` exit while waiting for keys. +#[test] +#[cfg(unix)] +fn test_piped_stdin_survives_writer_close() { + use std::fs::OpenOptions; + use std::io::pipe; + + // With a non-tty stdin, keys come from `/dev/tty`. Skip when it is absent + // (some sandboxes); GitHub CI has it. + if OpenOptions::new() + .read(true) + .write(true) + .open("/dev/tty") + .is_err() + { + return; + } + + let (path, mut controller, _replica) = pty_path(); + let (_at, mut ucmd) = at_and_ucmd!(); + let content: String = (0..80).map(|i| format!("line {i}\n")).collect(); + let (reader, mut writer) = pipe().unwrap(); + writer.write_all(content.as_bytes()).unwrap(); + drop(writer); // EOF / hangup on the data pipe only + + let mut child = ucmd + .set_stdin(reader) + .set_stdout(File::create(&path).unwrap()) + .args(&["-n", "5"]) + .run_no_wait(); + + child.delay(500); + assert!( + child.is_alive(), + "more exited after piped stdin closed; key tty should still be open" + ); + // Best-effort quit via display pty (keys are on /dev/tty); then kill. + let _ = controller.write_all(b"q"); + child.delay(50); + child.kill(); +} + #[test] #[cfg(unix)] fn test_lines_option_max_u16_no_overflow() {