diff options
| author | Pádraig Brady <P@draigBrady.com> | 2025-10-28 19:30:08 +0000 |
|---|---|---|
| committer | Pádraig Brady <P@draigBrady.com> | 2025-10-29 19:03:23 +0000 |
| commit | b294aff3fe6c8ebeda02b8c77877ba618474de41 (patch) | |
| tree | 0b511881d71ce69f229d09c165f98072f1a98397 | |
| parent | build: update gnulib submodule to latest (diff) | |
| download | coreutils-b294aff3fe6c8ebeda02b8c77877ba618474de41.tar.gz coreutils-b294aff3fe6c8ebeda02b8c77877ba618474de41.zip | |
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.
| -rw-r--r-- | NEWS | 4 | ||||
| -rw-r--r-- | src/sort.c | 89 | ||||
| -rwxr-xr-x | tests/sort/sort-compress-proc.sh | 26 |
3 files changed, 77 insertions, 42 deletions
@@ -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 diagnoses if it can't write more data to an + exited compressor. 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 |
