tac, tail, dd: detect closed stdin before Rust sanitizes it to /dev/null by ChrisDryden · Pull Request #9664 · uutils/coreutils

ChrisDryden · 2025-12-15T06:31:52Z

Using the same approach that was identified for the pipeline signals, we can retrieve the state of stdin and stdout before the rust runtime overrides the fd to /dev/null

This means that we will be able to both preserve the pre-existing behaviour for passing in /dev/null and also allow us to raise the same error message and code when the closed fd is passed to the stdin or stdout as the GNU implementation.

github-actions · 2025-12-15T06:48:32Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

github-actions · 2025-12-15T17:53:06Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

anastygnome · 2025-12-15T19:30:01Z

I'm not sure we should do this, the shell shouldn't allow that in the first place, this is not compliant with posix.
@sylvestre should we import such technical debt in our tools?
See rust-lang/rust#108139 (comment)

ChrisDryden · 2025-12-15T19:46:35Z

For tracking this would be the same approach of getting the PIPE signal as here: #9657
also has discussions on: #5906 (comment)

Maybe my understanding of the discussion is not complete, @anastygnome are you saying that this UUTILS implementation is not Posix compliant and that the GNU Coreutils is compliant in regards to this? Or are you saying that both the GNU Coreutils implementation and this implementation are non-compliant and that we should diverge from the GNU Coreutils implementation

codspeed-hq · 2025-12-15T20:36:25Z

CodSpeed Performance Report

Merging this PR will not alter performance

_{Comparing ChrisDryden:fix-stdin-closed-detection (9578fb5) with main (0b0ba60)}

Summary

✅ 141 untouched benchmarks
⏩ 38 skipped benchmarks¹

38 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. ↩

github-actions · 2025-12-15T20:51:59Z

GNU testsuite comparison:

Congrats! The gnu test tests/dd/stderr is no longer failing!
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

sylvestre · 2025-12-15T20:54:17Z

GNU testsuite comparison:

Congrats! The gnu test tests/dd/stderr is no longer failing!
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

impressive!

collinfunk · 2025-12-15T20:55:51Z

I'm not sure we should do this, the shell shouldn't allow that in the first place, this is not compliant with posix.
@sylvestre should we import such technical debt in our tools?

Where does POSIX disallow sh from closing file descriptors?

This behavior has been supported the Bourne shell for ages. Even Solaris /bin/sh supports it:

$ gyes >&-
gyes: standard output: Bad file number
gyes: write error: Bad file number

My personal opinion is that Rust really should have a way to disable the messing around it does with signal actions and file descriptors before main().

github-actions · 2025-12-15T21:19:45Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/dd/stderr is no longer failing!
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

anastygnome · 2025-12-15T21:30:46Z

For tracking this would be the same approach of getting the PIPE signal as here: #9657 also has discussions on: #5906 (comment)

Maybe my understanding of the discussion is not complete, @anastygnome are you saying that this UUTILS implementation is not Posix compliant and that the GNU Coreutils is compliant in regards to this? Or are you saying that both the GNU Coreutils implementation and this implementation are non-compliant and that we should diverge from the GNU Coreutils implementation

So, to recap the issue

Basically this point of the posix standard was ambiguous before 2017.
Which is why the shell supports it, and gnu coreutils had to work around it

Since 2017, the posix standard says that if any of the 3 standards file descriptiors are closed, we are not in a posix compliant environment.
Rust works around that by reopening said file descriptors on /dev/null.

So the question is :

Do we consider posix post 2017 as the new norm, or do we add this check as a continuing workaround ?

I'll quote the posix standard later but it has been there on older issues.

anastygnome · 2025-12-16T19:03:23Z

+#[cfg(unix)]
+#[allow(clippy::missing_safety_doc)]
+pub unsafe extern "C" fn capture_stdio_state() {
+    use nix::libc;
+    unsafe {
+        STDIN_WAS_CLOSED.store(
+            libc::fcntl(libc::STDIN_FILENO, libc::F_GETFD) == -1,
+            Ordering::Relaxed,
+        );
+        STDOUT_WAS_CLOSED.store(
+            libc::fcntl(libc::STDOUT_FILENO, libc::F_GETFD) == -1,
+            Ordering::Relaxed,
+        );
+        STDERR_WAS_CLOSED.store(
+            libc::fcntl(libc::STDERR_FILENO, libc::F_GETFD) == -1,
+            Ordering::Relaxed,
+        );
+    }
+}


For efficiency we should do something similar to the fastpath shown in this function. It is the function that reopens the FDs in the standard library.

how about this

Suggested change

#[cfg(unix)]

#[allow(clippy::missing_safety_doc)]

pub unsafe extern "C" fn capture_stdio_state() {

use nix::libc;

unsafe {

STDIN_WAS_CLOSED.store(

libc::fcntl(libc::STDIN_FILENO, libc::F_GETFD) == -1,

Ordering::Relaxed,

);

STDOUT_WAS_CLOSED.store(

libc::fcntl(libc::STDOUT_FILENO, libc::F_GETFD) == -1,

Ordering::Relaxed,

);

STDERR_WAS_CLOSED.store(

libc::fcntl(libc::STDERR_FILENO, libc::F_GETFD) == -1,

Ordering::Relaxed,

);

}

}

/// # Safety

///

/// Must be called very early in process startup, before the runtime may reopen

/// closed standard file descriptors. Uses low-level libc APIs.

/// This code is just checking the state of the three standard fds and has no side-effects.

#[cfg(unix)]

pub unsafe extern "C" fn capture_stdio_state() {

use nix::libc;

// Prefer poll() where it reliably reports POLLNVAL

#[cfg(not(any(

miri,

target_os = "emscripten",

target_os = "fuchsia",

target_os = "vxworks",

target_os = "macos",

target_os = "ios",

target_os = "watchos",

target_os = "redox",

target_os = "l4re",

target_os = "horizon",

)))]

{

unsafe {

let mut pfds = [

libc::pollfd {

fd: libc::STDIN_FILENO,

events: 0,

revents: 0,

},

libc::pollfd {

fd: libc::STDOUT_FILENO,

events: 0,

revents: 0,

},

libc::pollfd {

fd: libc::STDERR_FILENO,

events: 0,

revents: 0,

},

];

let ok = loop {

match libc::poll(pfds.as_mut_ptr(), pfds.len() as _, 0) {

-1 => match *libc::__errno_location() {

libc::EINTR => continue,

libc::EINVAL | libc::ENOMEM | libc::EAGAIN => break false,

_ => libc::abort(),

},

_ => break true,

}

};

if ok {

STDIN_WAS_CLOSED.store(pfds[0].revents & libc::POLLNVAL != 0, Ordering::Relaxed);

STDOUT_WAS_CLOSED.store(pfds[1].revents & libc::POLLNVAL != 0, Ordering::Relaxed);

STDERR_WAS_CLOSED.store(pfds[2].revents & libc::POLLNVAL != 0, Ordering::Relaxed);

return;

}

}

}

// Fallback: fcntl()

unsafe {

STDIN_WAS_CLOSED.store(

libc::fcntl(libc::STDIN_FILENO, libc::F_GETFD) == -1,

Ordering::Relaxed,

);

STDOUT_WAS_CLOSED.store(

libc::fcntl(libc::STDOUT_FILENO, libc::F_GETFD) == -1,

Ordering::Relaxed,

);

STDERR_WAS_CLOSED.store(

libc::fcntl(libc::STDERR_FILENO, libc::F_GETFD) == -1,

Ordering::Relaxed,

);

}

}

anastygnome · 2025-12-17T06:32:28Z

Also should be done for cat ;)

ChrisDryden · 2025-12-17T15:12:53Z

I'm open either way to the approach, was hoping to get a maintainers opinion on which approach they'd be interested in?

anastygnome · 2025-12-19T22:50:21Z

@sylvestre , what do you think?

ChrisDryden · 2025-12-23T18:54:04Z

@sylvestre any chance we could get your guidance on this general topic of reading signals before the rust runtime overrides them. I believe there's more than 20 separate issues in the queue related to this because if we use this approach we can read the original state of the file descriptors and read the different SIGPIPE signals correctly, and once we have a PR in that the maintainers are aligned on the approach for how to read these signals we can begin closing all of those issues:

To get them connected I will start collecting the issues that are solvable by the init_data approach of reading things, either through this PR or through the similar SIGPIPE PR: #9657

sylvestre · 2025-12-23T19:00:07Z

I would prefer we match what GNU is doing now :)

ChrisDryden · 2025-12-23T19:10:12Z

So the end behaviour with this signal handling is the only solution that I've seen so far that fully matches the end behaviour of the GNU utilities so far. We have a bunch of hacks previously with the stdin, stderr in specific utilities for specific edge cases but this one is the one that will match all of the behaviors

ChrisDryden · 2025-12-23T19:11:14Z

To say that this approach is the solution that handles matching the GNU implementation in all of these use cases as an example, theres many more in the issues folder:

Closed file descriptor handling:

SigPIPE signal handling:

anastygnome · 2025-12-23T20:45:01Z

@ChrisDryden So, after checking in an env and spinning lots of uutils in parallel, I've seen performance degradation when doing 3 syscalls (this PR), vs my solution.

Will post the results here shortly but we kinda want this, especially in cases where the system is already under load, but we are able to use poll()

sylvestre · 2025-12-23T22:19:14Z

    pub(crate) fn print_io_lines(&self) {
        let mut stderr = std::io::stderr();
-        self.write_io_lines(&mut stderr).unwrap();
+        if self.write_io_lines(&mut stderr).is_err() {


i would have preferred to see this change into a different pr

Good idea, splitting it now

ChrisDryden · 2025-12-30T18:40:27Z

@anastygnome Do you have those results? I'm a bit suprised that the syscalls would be noticed in instantiation relative to the amount of overhead that exists, iirc in these utilities >33% of the time in the LD_PRELOAD stuff and 33% is the translation loading so I would have thought that the additional syscall's would be negligible.

anastygnome · 2025-12-31T13:35:30Z

Will provide them when I have access to the machine, but the situation where you'd get the most difference is a heavy loaded system (> 80% CPU usage on all cores,> 90% ram usage , Io bottleneck running on another process) and you start using a bash script. One of the 3 syscalls ended up waiting for the os.

With the poll version, the problem was not present as much.

ChrisDryden · 2026-01-10T16:37:09Z

Going to try and get the ball rolling on this today, I'll make a new PR with the alternative approach and not apply any of the fixes that would use it to minimize the size of the PR.

sylvestre · 2026-01-13T21:12:25Z

needs to be rebased

github-actions · 2026-01-14T00:36:56Z

GNU testsuite comparison:

Congrats! The gnu test tests/dd/stderr is no longer failing!
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

mfontanaar · 2026-01-18T16:52:15Z

Going to try and get the ball rolling on this today, I'll make a new PR with the alternative approach and not apply any of the fixes that would use it to minimize the size of the PR.

Were you able to start that work?

I worked for a couple of hours on this (I don't think I pushed anything to a public fork as I was still iterating over different designs), so happy to review if you want. Just tag me in the PR.

sylvestre · 2026-01-19T21:43:28Z

please ping me when it is ready! 3 GNU tests fixed is a big deal :)

anastygnome · 2026-01-19T22:22:51Z

@sylvestre you can merge as is, we can optimise later.

sylvestre · 2026-01-19T22:29:39Z

thanks!

ChrisDryden · 2026-01-19T22:37:21Z

Sweet! This actually unblocks three more GNU tests, will open them now

ChrisDryden · 2026-01-19T22:42:30Z

This should unblock:
misc/close-stdout.sh
misc/write-errors.sh

ChrisDryden changed the title ~~fix: detect closed stdin before Rust sanitizes it to /dev/null~~ WIP fix: detect closed stdin before Rust sanitizes it to /dev/null Dec 15, 2025

ChrisDryden mentioned this pull request Dec 15, 2025

GNU coreutils 9.9: Test report #9127

Closed

ChrisDryden force-pushed the fix-stdin-closed-detection branch from 32ab405 to e0bafa3 Compare December 15, 2025 17:04

ChrisDryden changed the title ~~WIP fix: detect closed stdin before Rust sanitizes it to /dev/null~~ tac, tail: detect closed stdin before Rust sanitizes it to /dev/null Dec 15, 2025

ChrisDryden marked this pull request as ready for review December 15, 2025 17:05

ChrisDryden force-pushed the fix-stdin-closed-detection branch from e0bafa3 to bfdfe19 Compare December 15, 2025 17:10

This was referenced Dec 15, 2025

tac: should fail when stdin is closed but doesn't #2873

Closed

dd: should return 1 on closed stderr but returns 0 #5906

Closed

ChrisDryden force-pushed the fix-stdin-closed-detection branch from bfdfe19 to d61c41f Compare December 15, 2025 20:27

ChrisDryden changed the title ~~tac, tail: detect closed stdin before Rust sanitizes it to /dev/null~~ tac, tail, dd: detect closed stdin before Rust sanitizes it to /dev/null Dec 15, 2025

ChrisDryden force-pushed the fix-stdin-closed-detection branch from d61c41f to dfabb4b Compare December 15, 2025 20:35

fix: detect closed stdin before Rust sanitizes it to /dev/null

2c10c5f

ChrisDryden force-pushed the fix-stdin-closed-detection branch from dfabb4b to 2c10c5f Compare December 15, 2025 21:02

anastygnome suggested changes Dec 16, 2025

View reviewed changes

ChrisDryden mentioned this pull request Dec 19, 2025

GNU coreutils 9.9: Detailed Test report 12/19 #9729

Closed

sylvestre reviewed Dec 23, 2025

View reviewed changes

ChrisDryden mentioned this pull request Dec 26, 2025

dd: handle stderr write errors gracefully in progress output #9852

Closed

Merge branch 'main' into fix-stdin-closed-detection

4538bf0

ChrisDryden mentioned this pull request Jan 10, 2026

Discussion: Guidelines for printing to stdout #3093

Open

Merge main into fix-stdin-closed-detection

9578fb5

ChrisDryden mentioned this pull request Jan 19, 2026

stty: unshare -rm sh -c "mount --bind /mnt /dev && stty <&-" SIGABRT #10236

Closed

sylvestre merged commit 2d7a3bf into uutils:main Jan 19, 2026
132 checks passed

max-amb mentioned this pull request Jan 25, 2026

GNU coreutils 9.9: Detailed Test report 01/20 #10390

Closed

ChrisDryden mentioned this pull request Jan 28, 2026

dd: should return 1 when stderr is redirected to /dev/full but returns 101 #5907

Closed

moonfruit mentioned this pull request Feb 3, 2026

uutils-selected 0.6.0 moonfruit/homebrew-tap#453

Closed

-#[cfg(unix)]
-#[allow(clippy::missing_safety_doc)]
-pub unsafe extern "C" fn capture_stdio_state() {
-    use nix::libc;
-    unsafe {
-        STDIN_WAS_CLOSED.store(
-            libc::fcntl(libc::STDIN_FILENO, libc::F_GETFD) == -1,
-            Ordering::Relaxed,
-        );
-        STDOUT_WAS_CLOSED.store(
-            libc::fcntl(libc::STDOUT_FILENO, libc::F_GETFD) == -1,
-            Ordering::Relaxed,
-        );
-        STDERR_WAS_CLOSED.store(
-            libc::fcntl(libc::STDERR_FILENO, libc::F_GETFD) == -1,
-            Ordering::Relaxed,
-        );
-    }
-}
+/// # Safety
+///
+/// Must be called very early in process startup, before the runtime may reopen
+/// closed standard file descriptors. Uses low-level libc APIs.
+/// This code is just checking the state of the three standard fds and has no side-effects.
+#[cfg(unix)]
+pub unsafe extern "C" fn capture_stdio_state() {
+    use nix::libc;
+    // Prefer poll() where it reliably reports POLLNVAL
+    #[cfg(not(any(
+        miri,
+        target_os = "emscripten",
+        target_os = "fuchsia",
+        target_os = "vxworks",
+        target_os = "macos",
+        target_os = "ios",
+        target_os = "watchos",
+        target_os = "redox",
+        target_os = "l4re",
+        target_os = "horizon",
+    )))]
+    {
+        unsafe {
+            let mut pfds = [
+                libc::pollfd {
+                    fd: libc::STDIN_FILENO,
+                    events: 0,
+                    revents: 0,
+                },
+                libc::pollfd {
+                    fd: libc::STDOUT_FILENO,
+                    events: 0,
+                    revents: 0,
+                },
+                libc::pollfd {
+                    fd: libc::STDERR_FILENO,
+                    events: 0,
+                    revents: 0,
+                },
+            ];
+            let ok = loop {
+                match libc::poll(pfds.as_mut_ptr(), pfds.len() as _, 0) {
+                    -1 => match *libc::__errno_location() {
+                        libc::EINTR  => continue,
+                        libc::EINVAL | libc::ENOMEM | libc::EAGAIN  => break false,
+                        _ => libc::abort(),
+                    },
+                    _ => break true,
+                }
+            };
+            if ok {
+                STDIN_WAS_CLOSED.store(pfds[0].revents & libc::POLLNVAL != 0, Ordering::Relaxed);
+                STDOUT_WAS_CLOSED.store(pfds[1].revents & libc::POLLNVAL != 0, Ordering::Relaxed);
+                STDERR_WAS_CLOSED.store(pfds[2].revents & libc::POLLNVAL != 0, Ordering::Relaxed);
+                return;
+            }
+        }
+    }
+    // Fallback: fcntl()
+    unsafe {
+        STDIN_WAS_CLOSED.store(
+            libc::fcntl(libc::STDIN_FILENO, libc::F_GETFD) == -1,
+            Ordering::Relaxed,
+        );
+        STDOUT_WAS_CLOSED.store(
+            libc::fcntl(libc::STDOUT_FILENO, libc::F_GETFD) == -1,
+            Ordering::Relaxed,
+        );
+        STDERR_WAS_CLOSED.store(
+            libc::fcntl(libc::STDERR_FILENO, libc::F_GETFD) == -1,
+            Ordering::Relaxed,
+        );
+    }
+}

Uh oh!

Uh oh!

Conversation

ChrisDryden commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Dec 15, 2025

Uh oh!

github-actions Bot commented Dec 15, 2025

Uh oh!

anastygnome commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrisDryden commented Dec 15, 2025

Uh oh!

codspeed-hq Bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging this PR will not alter performance

Summary

Footnotes

Uh oh!

github-actions Bot commented Dec 15, 2025

Uh oh!

sylvestre commented Dec 15, 2025

Uh oh!

collinfunk commented Dec 15, 2025

Uh oh!

github-actions Bot commented Dec 15, 2025

Uh oh!

anastygnome commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anastygnome Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anastygnome commented Dec 17, 2025

Uh oh!

ChrisDryden commented Dec 17, 2025

Uh oh!

anastygnome commented Dec 19, 2025

Uh oh!

ChrisDryden commented Dec 23, 2025

Uh oh!

sylvestre commented Dec 23, 2025

Uh oh!

ChrisDryden commented Dec 23, 2025

Uh oh!

ChrisDryden commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anastygnome commented Dec 23, 2025

Uh oh!

sylvestre Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisDryden Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisDryden commented Dec 30, 2025

Uh oh!

anastygnome commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrisDryden commented Jan 10, 2026

Uh oh!

sylvestre commented Jan 13, 2026

Uh oh!

github-actions Bot commented Jan 14, 2026

Uh oh!

mfontanaar commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sylvestre commented Jan 19, 2026

Uh oh!

anastygnome commented Jan 19, 2026

Uh oh!

Uh oh!

sylvestre commented Jan 19, 2026

ChrisDryden commented Dec 15, 2025 •

edited

Loading

anastygnome commented Dec 15, 2025 •

edited

Loading

codspeed-hq Bot commented Dec 15, 2025 •

edited

Loading

anastygnome commented Dec 15, 2025 •

edited

Loading

anastygnome Dec 16, 2025 •

edited

Loading

ChrisDryden commented Dec 23, 2025 •

edited

Loading

anastygnome commented Dec 31, 2025 •

edited

Loading

mfontanaar commented Jan 18, 2026 •

edited

Loading