more: exit when the key tty hangs up by Franklin-Qi · Pull Request #15264 · uutils/coreutils

Franklin-Qi · 2026-10-12T09:18:52Z

Enable crossterm use-dev-tty so EOF is not a tight mio read loop, and leave the event loop when stdin/stdout report POLLHUP.

Closes: #15258

sylvestre · 2026-10-12T11:29:57Z

clippy is sad on 4 jobs :)

rustfmt is sad too :)
could you please run cargo fmt?

Diff in /home/runner/work/coreutils/coreutils/tests/by-util/test_more.rs:330:
     // When -n is u16::MAX (65535), adding 1 for the banner line used to
     // overflow. Verify that more starts and displays content without crashing.
     let (child, mut controller, output) =
-        run_more_with_pty(&["-n", "65535"], "test.txt", "line1\nline2\nline3\n");    assert!(output.contains("line1"));
+        run_more_with_pty(&["-n", "65535"], "test.txt", "line1\nline2\nline3\n");
+    assert!(output.contains("line1"));
     quit_more(&mut controller, child);
 }

##[endgroup]
     Removed 0 files
##[endgroup]
##[error]ssh exited with code 1
Post job cleanup.
[command]/usr/bin/git version
git version 2.55.0
Temporarily overriding HOME='/home/runner/work/_temp/a051b0d1-aa2a-4dd8-abf1-87cc508d6e57' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/usr/bin/git config --global --add safe.directory /home/runner/work/coreutils/coreutils

sylvestre · 2026-10-12T11:30:12Z

    if stdout.is_terminal() {
-        stdout.queue(Show)?.queue(LeaveAlternateScreen)?;
-        terminal::disable_raw_mode()?;
+        let _ = stdout.queue(Show);


why ignore all errors now, even in the normal (non hangup) path?

sylvestre · 2026-10-12T11:30:18Z

+            revents: 0,
+        };
+        // POLLHUP/ERR appear in revents even when `events` is 0.
+        let n = unsafe { libc::poll(std::ptr::from_mut(&mut pfd), 1, 0) };


unsafe without a // SAFETY: comment. could we use nix/rustix poll instead of adding libc?

sylvestre · 2026-10-12T11:30:20Z

+        n >= 0 && (pfd.revents & (libc::POLLHUP | libc::POLLERR | libc::POLLNVAL)) != 0
+    }
+
+    fd_hung_up(stdin().as_raw_fd()) || fd_hung_up(stdout().as_raw_fd())


stdin is the data pipe with cat file | more, it will report POLLHUP as soon as cat exits and more will quit right away, no?
please check the tty more actually reads keys from, and add a test for the piped case

sylvestre · 2026-10-12T11:30:25Z

+                let _ = reset_term();
+                return Ok(());
+            }
+            let Ok(ready) = event::poll(Duration::from_millis(100)) else {


this turns any poll/read error into a silent success, is it really what we want?

sylvestre · 2026-10-12T11:30:30Z

+    );
+
+    let output = Command::new("python3").arg("-c").arg(script).output();
+    let Ok(output) = output else {


silently passing when python3 is missing isn't great. could it use the existing pty helpers instead of an inline python script?

sylvestre · 2026-10-12T11:30:33Z

    let (child, mut controller, output) =
-        run_more_with_pty(&["-n", "65535"], "test.txt", "line1\nline2\nline3\n");
-    assert!(output.contains("line1"));
+        run_more_with_pty(&["-n", "65535"], "test.txt", "line1\nline2\nline3\n");    assert!(output.contains("line1"));


broken formatting + unrelated change, please run cargo fmt

Detect hangup on the same fd crossterm uses for keys (stdin if it was a tty at startup, otherwise /dev/tty), without treating a closed content pipe as keyboard EOF. Avoid crossterm's timed poll busy-loop by using a zero-timeout poll plus a short sleep. Closes: uutils#15258

codspeed · 2026-10-16T10:09:18Z

Merging this PR will regress 1 benchmark

⚠️

118 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

du_deep_tree[(100, 3)]

cksum_multiple_files

du_wide_tree[(5000, 500)]

sort_unique_utf8_locale

cksum_raw_output

cksum_crc

dd_copy_64k_blocks

check_sorted_utf8_locale

du_all_wide_tree[(5000, 500)]

cksum_sysv

…and 108 more

ℹ️ 1 benchmark has no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

sort_dictionary_order[500000]

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 399 untouched benchmarks
⏩ 70 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`factor_five_38_bit_primes`	1.9 s	2.1 s	-10.78%
⚡	Simulation	`factor_three_39_bit_primes`	598.6 ms	473.9 ms	+26.3%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing Franklin-Qi:fix/15258-more-hangup-busy-loop (734e47a) with main (dccea81)²}

70 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (781b499) during the generation of this report, so dccea81 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

sylvestre reviewed Oct 12, 2026

View reviewed changes

Comment thread src/uu/more/Cargo.toml Outdated

sylvestre reviewed Oct 12, 2026

View reviewed changes

Comment thread src/uu/more/src/more.rs Outdated

sylvestre reviewed Oct 12, 2026

View reviewed changes

Franklin-Qi force-pushed the fix/15258-more-hangup-busy-loop branch 2 times, most recently from 967e9ae to 7c4117b Compare October 16, 2026 09:14

Franklin-Qi force-pushed the fix/15258-more-hangup-busy-loop branch from 7c4117b to 734e47a Compare October 16, 2026 09:15

Navigation Menu

Repository navigation

more: exit when the key tty hangs up - #15264

more: exit when the key tty hangs up#15264
Franklin-Qi wants to merge 1 commit into
uutils:mainfrom
Franklin-Qi:fix/15258-more-hangup-busy-loop

Franklin-Qi commented Oct 12, 2026

Uh oh!

sylvestre commented Oct 12, 2026

Uh oh!

Uh oh!

Uh oh!

sylvestre Oct 12, 2026

Uh oh!

sylvestre Oct 12, 2026

Uh oh!

sylvestre Oct 12, 2026

Uh oh!

sylvestre Oct 12, 2026

Uh oh!

sylvestre Oct 12, 2026

Uh oh!

sylvestre Oct 12, 2026

Uh oh!

codspeed Bot commented Oct 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Franklin-Qi commented Oct 12, 2026

Uh oh!

sylvestre commented Oct 12, 2026

Uh oh!

Uh oh!

Uh oh!

sylvestre Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

sylvestre Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

sylvestre Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

sylvestre Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

sylvestre Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

sylvestre Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 16, 2026

Merging this PR will regress 1 benchmark

Performance Changes

Footnotes

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants