On 28/10/2025 17:33, Pádraig Brady wrote:
I've only thought about this for a couple of minutes but the attached could
work,
where we always ignore SIGPIPE and then convert the EPIPE on stdout to SIGPIPE.
I'll do lots of testing with this and add a new test before pushing,
but it does pass basic tests here:
# ---- put previous exit statuses in prompt
$ PS1="[\[\033[1;31m\]\${PIPESTATUS[@]/#0/\[\033[0m\]"\
"\[\033[1;32m\]0\[\033[1;31m\]}\[\033[0m\]] \w$"
[0] $ printf '%s\n' '#!/bin/sh' 'exit 1' > exit1 && chmod a+x exit1
# ---- we now diagnose these failures always
[0] $ src/sort -S 100K --compress-program=./exit1 Makefile | head -n1
sort: write failed: /tmp/sortlh70wt: Broken pipe
# ---- normal pipe operation is maintained
[2 0] $ seq 10000 | sort | :
[0 141 0] $
That idea seems to work, but I needed to tweak to
ensure we cleanup() to remove temp files upon receiving the SIGPIPE.
I'll apply attached patch tomorrow.
cheers,
Padraig
From ae16a487be34dc997aed63667c5e20e44718ea82 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Tue, 28 Oct 2025 19:30:08 +0000
Subject: [PATCH] sort: fix silent exit upon SIGPIPE from --compress-program
* src/sort.c (main): Ignore SIGPIPE so we've more control over
how we handle for stdout and compression programs.
(sort_die): Handle EPIPE from stdout and mimic a standard SIGPIPE,
otherwise reverting to a standard exit(SORT_FAILURE);
* tests/sort/sort-compress-proc.sh: Add a test case.
* NEWS: Mention the bug fix.
---
NEWS | 4 ++
src/sort.c | 89 ++++++++++++++++++--------------
tests/sort/sort-compress-proc.sh | 26 ++++++++--
3 files changed, 77 insertions(+), 42 deletions(-)
diff --git a/NEWS b/NEWS
index 96b54e108..29c06aa61 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,10 @@ GNU coreutils NEWS -*- outline -*-
Although these directories are nonempty, 'rmdir DIR' succeeds on them.
[bug introduced in coreutils-8.16]
+ 'sort --compress-program' now always diagnoses if a compressor exits before
+ reading all data. Previously sort could have exited silently in this case.
+ [bug introduced in coreutils-6.8]
+
'tail' outputs the correct number of lines again for non-small -n values.
Previously it may have output too few lines.
[bug introduced in coreutils-9.8]
diff --git a/src/sort.c b/src/sort.c
index 7127f671b..78b2f69f9 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -371,12 +371,57 @@ static bool debug;
number are present, temp files will be used. */
static unsigned int nmerge = NMERGE_DEFAULT;
+/* Whether SIGPIPE had the default disposition at startup. */
+static bool default_SIGPIPE;
+
+/* The list of temporary files. */
+struct tempnode
+{
+ struct tempnode *volatile next;
+ pid_t pid; /* The subprocess PID; undefined if state == UNCOMPRESSED. */
+ char state;
+ char name[FLEXIBLE_ARRAY_MEMBER];
+};
+static struct tempnode *volatile temphead;
+static struct tempnode *volatile *temptail = &temphead;
+
+/* Clean up any remaining temporary files. */
+
+static void
+cleanup (void)
+{
+ struct tempnode const *node;
+
+ for (node = temphead; node; node = node->next)
+ unlink (node->name);
+ temphead = nullptr;
+}
+
+/* Handle interrupts and hangups. */
+
+static void
+sighandler (int sig)
+{
+ if (! SA_NOCLDSTOP)
+ signal (sig, SIG_IGN);
+
+ cleanup ();
+
+ signal (sig, SIG_DFL);
+ raise (sig);
+}
+
/* Report MESSAGE for FILE, then clean up and exit.
If FILE is null, it represents standard output. */
static void
sort_die (char const *message, char const *file)
{
+ /* If we got EPIPE writing to stdout (from a previous fwrite() or fclose()
+ and SIGPIPE was originally SIG_DFL, mimic standard SIGPIPE behavior. */
+ if (errno == EPIPE && !file && default_SIGPIPE)
+ sighandler (SIGPIPE);
+
error (SORT_FAILURE, errno, "%s: %s", message,
quotef (file ? file : _("standard output")));
}
@@ -631,17 +676,6 @@ cs_leave (struct cs_status const *status)
the subprocess to finish. */
enum { UNCOMPRESSED, UNREAPED, REAPED };
-/* The list of temporary files. */
-struct tempnode
-{
- struct tempnode *volatile next;
- pid_t pid; /* The subprocess PID; undefined if state == UNCOMPRESSED. */
- char state;
- char name[FLEXIBLE_ARRAY_MEMBER];
-};
-static struct tempnode *volatile temphead;
-static struct tempnode *volatile *temptail = &temphead;
-
/* A file to be sorted. */
struct sortfile
{
@@ -780,18 +814,6 @@ reap_all (void)
reap (-1);
}
-/* Clean up any remaining temporary files. */
-
-static void
-cleanup (void)
-{
- struct tempnode const *node;
-
- for (node = temphead; node; node = node->next)
- unlink (node->name);
- temphead = nullptr;
-}
-
/* Cleanup actions to take when exiting. */
static void
@@ -4262,20 +4284,6 @@ parse_field_count (char const *string, size_t *val, char const *msgid)
return suffix;
}
-/* Handle interrupts and hangups. */
-
-static void
-sighandler (int sig)
-{
- if (! SA_NOCLDSTOP)
- signal (sig, SIG_IGN);
-
- cleanup ();
-
- signal (sig, SIG_DFL);
- raise (sig);
-}
-
/* Set the ordering options for KEY specified in S.
Return the address of the first character in S that
is not a valid ordering option.
@@ -4409,7 +4417,7 @@ main (int argc, char **argv)
static int const sig[] =
{
/* The usual suspects. */
- SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
+ SIGALRM, SIGHUP, SIGINT, SIGQUIT, SIGTERM,
#ifdef SIGPOLL
SIGPOLL,
#endif
@@ -4457,6 +4465,11 @@ main (int argc, char **argv)
}
signal (SIGCHLD, SIG_DFL); /* Don't inherit CHLD handling from parent. */
+ /* Ignore SIGPIPE so write failures are reported via EPIPE errno.
+ For stdout, sort_die() will reraise SIGPIPE if it was originally SIG_DFL.
+ For compression pipes, sort_die() will exit with SORT_FAILURE. */
+ default_SIGPIPE = (signal (SIGPIPE, SIG_IGN) == SIG_DFL);
+
/* The signal mask is known, so it is safe to invoke exit_cleanup. */
atexit (exit_cleanup);
diff --git a/tests/sort/sort-compress-proc.sh b/tests/sort/sort-compress-proc.sh
index c410b7fce..203342924 100755
--- a/tests/sort/sort-compress-proc.sh
+++ b/tests/sort/sort-compress-proc.sh
@@ -17,7 +17,7 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
-print_ver_ sort
+print_ver_ sort kill
expensive_
# Terminate any background processes
@@ -25,9 +25,9 @@ cleanup_() { kill $pid 2>/dev/null && wait $pid; }
SORT_FAILURE=2
-seq -w 2000 > exp || fail=1
-tac exp > in || fail=1
-insize=$(stat -c %s - <in) || fail=1
+seq -w 2000 > exp || framework_failure_
+tac exp > in || framework_failure_
+insize=$(stat -c %s - <in) || framework_failure_
# This compressor's behavior is adjustable via environment variables.
export PRE_COMPRESS=
@@ -42,6 +42,24 @@ EOF
chmod +x compress
+# "Early exit" test
+#
+# In this test, the compressor exits before reading all (any) data.
+# Until coreutils 9.9 'sort' could get a SIGPIPE writing to the
+# exited processes and silently exit. Note the same issue can happen
+# irrespective of exit status. It's more likely to happen in the
+# case of the child exiting with success, and if we write more data
+# (hence the --batch-size=30 and double "in"). Note we check sort doesn't
+# get SIGPIPE rather than if it returns SORT_FAILURE, because there is
+# the theoretical possibility that the kernel could buffer the
+# amount of data we're writing here and not issue the EPIPE to sort.
+# In other words we currently may not detect failures in the extreme edge case
+# of writing a small amount of data to a compressor that exits 0
+# while not reading all the data presented.
+PRE_COMPRESS='exit 0' \
+ sort --compress-program=./compress -S 1k --batch-size=30 ./in ./in > out
+test $(env kill -l $?) = 'PIPE' && fail=1
+
# "Impatient exit" tests
#
# In these test cases, the biggest compressor (or decompressor) exits
--
2.51.0
