coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

fix(yes): handle trapped SIGPIPE correctly

Open naoNao89 opened this issue 3 weeks ago • 12 comments

When SIGPIPE is trapped (trap '' PIPE), write fails with EPIPE instead of signal. Print diagnostic message to stderr like GNU coreutils, instead of silently exiting.

Test with trapped SIGPIPE (the issue scenario)

$ (trap '' PIPE && ./target/debug/yes | :) 2>&1
yes: stdout: Broken pipe

Compare with GNU yes

$ (trap '' PIPE && /usr/bin/yes | :) 2>&1  
yes: stdout: Broken pipe  (matches)

Refs: POSIX Signal specs, GNU Signal handling

Fixes #7252

naoNao89 avatar Dec 04 '25 20:12 naoNao89

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

github-actions[bot] avatar Dec 04 '25 20:12 github-actions[bot]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

github-actions[bot] avatar Dec 04 '25 21:12 github-actions[bot]

While it looks correct in the case you list, it doesn't fix the handling of SIGPIPE.

See the following example:

$ yes | head -n 1
y
$ echo ${PIPESTATUS[0]} ${PIPESTATUS[1]} 
141 0
$ ./target/debug/yes | head -n 1
y
yes: stdout: Broken pipe
$ echo ${PIPESTATUS[0]} ${PIPESTATUS[1]} 
0 0

collinfunk avatar Dec 05 '25 02:12 collinfunk

$ echo ${PIPESTATUS[0]} ${PIPESTATUS[1]}
$ ./target/debug/yes | head -n 1
y

naoNao89 avatar Dec 05 '25 06:12 naoNao89

CodSpeed Performance Report

Merging #9560 will not alter performance

Comparing naoNao89:fix-yes-sigpipe-trap-7252 (1ffaedd) with main (7c62885)

Summary

✅ 127 untouched
⏩ 6 skipped[^skipped]

[^skipped]: 6 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.

codspeed-hq[bot] avatar Dec 05 '25 06:12 codspeed-hq[bot]

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

github-actions[bot] avatar Dec 05 '25 06:12 github-actions[bot]

This looks incorrect. POSIX states [1]:

Signals set to the default action (SIG_DFL) in the calling process image shall be set to the default action in the new process image. Except for SIGCHLD, signals set to be ignored (SIG_IGN) by the calling process image shall be set to be ignored by the new process image.

If yes is created by a process where SIGPIPE is ignored, setting it to it's default would be incorrect.

See the following example:

$ env --ignore-signal=PIPE strace -o output-gnu yes | :
yes: standard output: Broken pipe
$ env --ignore-signal=PIPE strace -o output-uutils ./target/debug/yes | :
$ grep -F 'killed by SIGPIPE' output-gnu output-uutils 
output-uutils:+++ killed by SIGPIPE +++

So, I don't think this issue is fixable unless Rust has a way to remove the signal (SIGPIPE, SIG_IGN) call they add at startup.

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/exec.html

collinfunk avatar Dec 05 '25 08:12 collinfunk

the rust problem

Rust sets SIGPIPE = SIG_IGN by default at startup, making it impossible to distinguish:

  1. Parent explicitly set SIG_IGN (must be preserved per POSIX)
  2. Rust's default SIG_IGN (could theoretically be changed)

naoNao89 avatar Dec 06 '25 11:12 naoNao89

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

github-actions[bot] avatar Dec 06 '25 12:12 github-actions[bot]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

github-actions[bot] avatar Dec 07 '25 03:12 github-actions[bot]

I still don't think this is correct. The error should not be printed in this case, and the exit status of yes should not be zero:

$ ./target/debug/yes  | head -n 1
y
yes: stdout: Broken pipe
$ echo ${PIPESTATUS[0]} ${PIPESTATUS[1]} 
0 0

Here is the correct behavior:

$ yes | head -n 1
y
$ echo ${PIPESTATUS[0]} ${PIPESTATUS[1]} 
141 0

This is an unfortunate limitation of Rust, that requires a feature to be stabilized [1].

It also appears that unless you use exec* in an unsafe block, Rust's library will create child processes that do not inherit the signal action for SIGPIPE from the parent [2].

[1] https://dev-doc.rust-lang.org/beta/unstable-book/language-features/unix-sigpipe.html#unix_sigpipe--sig_ign [2] https://dev-doc.rust-lang.org/beta/unstable-book/language-features/unix-sigpipe.html#note-on-child-processes

collinfunk avatar Dec 07 '25 09:12 collinfunk

don't worry, i tested it on Rust nightly, and it's working. Hope for the next version

naoNao89 avatar Dec 09 '25 18:12 naoNao89

This can be fixed with the approach in https://github.com/uutils/coreutils/pull/9657

ChrisDryden avatar Dec 22 '25 16:12 ChrisDryden