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.

diff --git a/NEWS b/NEWS
index 96b54e108b5875b6b9ddda861194c296711f5a41..0cc760aeedae31ed2216553d49263ff638046f4d 100644 (file)
--- 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 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 7127f671b63d155cbd7e949146e719e2301551eb..78b2f69f96eca6419971ce57cd2691e0d607bfa6 100644 (file)
--- 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 c410b7fce0a766687eac8b4909576db953a7fb3b..20334292444c6a2aa8e976809bba5a976c824a12 100755 (executable)
--- 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