Skip to content

Commit b294aff

Browse files
committed
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.
1 parent 7bd932c commit b294aff

3 files changed

Lines changed: 77 additions & 42 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ GNU coreutils NEWS -*- outline -*-
2828
Although these directories are nonempty, 'rmdir DIR' succeeds on them.
2929
[bug introduced in coreutils-8.16]
3030

31+
'sort --compress-program' now diagnoses if it can't write more data to an
32+
exited compressor. Previously sort could have exited silently in this case.
33+
[bug introduced in coreutils-6.8]
34+
3135
'tail' outputs the correct number of lines again for non-small -n values.
3236
Previously it may have output too few lines.
3337
[bug introduced in coreutils-9.8]

src/sort.c

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -371,12 +371,57 @@ static bool debug;
371371
number are present, temp files will be used. */
372372
static unsigned int nmerge = NMERGE_DEFAULT;
373373

374+
/* Whether SIGPIPE had the default disposition at startup. */
375+
static bool default_SIGPIPE;
376+
377+
/* The list of temporary files. */
378+
struct tempnode
379+
{
380+
struct tempnode *volatile next;
381+
pid_t pid; /* The subprocess PID; undefined if state == UNCOMPRESSED. */
382+
char state;
383+
char name[FLEXIBLE_ARRAY_MEMBER];
384+
};
385+
static struct tempnode *volatile temphead;
386+
static struct tempnode *volatile *temptail = &temphead;
387+
388+
/* Clean up any remaining temporary files. */
389+
390+
static void
391+
cleanup (void)
392+
{
393+
struct tempnode const *node;
394+
395+
for (node = temphead; node; node = node->next)
396+
unlink (node->name);
397+
temphead = nullptr;
398+
}
399+
400+
/* Handle interrupts and hangups. */
401+
402+
static void
403+
sighandler (int sig)
404+
{
405+
if (! SA_NOCLDSTOP)
406+
signal (sig, SIG_IGN);
407+
408+
cleanup ();
409+
410+
signal (sig, SIG_DFL);
411+
raise (sig);
412+
}
413+
374414
/* Report MESSAGE for FILE, then clean up and exit.
375415
If FILE is null, it represents standard output. */
376416

377417
static void
378418
sort_die (char const *message, char const *file)
379419
{
420+
/* If we got EPIPE writing to stdout (from a previous fwrite() or fclose()
421+
and SIGPIPE was originally SIG_DFL, mimic standard SIGPIPE behavior. */
422+
if (errno == EPIPE && !file && default_SIGPIPE)
423+
sighandler (SIGPIPE);
424+
380425
error (SORT_FAILURE, errno, "%s: %s", message,
381426
quotef (file ? file : _("standard output")));
382427
}
@@ -631,17 +676,6 @@ cs_leave (struct cs_status const *status)
631676
the subprocess to finish. */
632677
enum { UNCOMPRESSED, UNREAPED, REAPED };
633678

634-
/* The list of temporary files. */
635-
struct tempnode
636-
{
637-
struct tempnode *volatile next;
638-
pid_t pid; /* The subprocess PID; undefined if state == UNCOMPRESSED. */
639-
char state;
640-
char name[FLEXIBLE_ARRAY_MEMBER];
641-
};
642-
static struct tempnode *volatile temphead;
643-
static struct tempnode *volatile *temptail = &temphead;
644-
645679
/* A file to be sorted. */
646680
struct sortfile
647681
{
@@ -780,18 +814,6 @@ reap_all (void)
780814
reap (-1);
781815
}
782816

783-
/* Clean up any remaining temporary files. */
784-
785-
static void
786-
cleanup (void)
787-
{
788-
struct tempnode const *node;
789-
790-
for (node = temphead; node; node = node->next)
791-
unlink (node->name);
792-
temphead = nullptr;
793-
}
794-
795817
/* Cleanup actions to take when exiting. */
796818

797819
static void
@@ -4262,20 +4284,6 @@ parse_field_count (char const *string, size_t *val, char const *msgid)
42624284
return suffix;
42634285
}
42644286

4265-
/* Handle interrupts and hangups. */
4266-
4267-
static void
4268-
sighandler (int sig)
4269-
{
4270-
if (! SA_NOCLDSTOP)
4271-
signal (sig, SIG_IGN);
4272-
4273-
cleanup ();
4274-
4275-
signal (sig, SIG_DFL);
4276-
raise (sig);
4277-
}
4278-
42794287
/* Set the ordering options for KEY specified in S.
42804288
Return the address of the first character in S that
42814289
is not a valid ordering option.
@@ -4409,7 +4417,7 @@ main (int argc, char **argv)
44094417
static int const sig[] =
44104418
{
44114419
/* The usual suspects. */
4412-
SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
4420+
SIGALRM, SIGHUP, SIGINT, SIGQUIT, SIGTERM,
44134421
#ifdef SIGPOLL
44144422
SIGPOLL,
44154423
#endif
@@ -4457,6 +4465,11 @@ main (int argc, char **argv)
44574465
}
44584466
signal (SIGCHLD, SIG_DFL); /* Don't inherit CHLD handling from parent. */
44594467

4468+
/* Ignore SIGPIPE so write failures are reported via EPIPE errno.
4469+
For stdout, sort_die() will reraise SIGPIPE if it was originally SIG_DFL.
4470+
For compression pipes, sort_die() will exit with SORT_FAILURE. */
4471+
default_SIGPIPE = (signal (SIGPIPE, SIG_IGN) == SIG_DFL);
4472+
44604473
/* The signal mask is known, so it is safe to invoke exit_cleanup. */
44614474
atexit (exit_cleanup);
44624475

tests/sort/sort-compress-proc.sh

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@
1717
# along with this program. If not, see <https://www.gnu.org/licenses/>.
1818

1919
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
20-
print_ver_ sort
20+
print_ver_ sort kill
2121
expensive_
2222

2323
# Terminate any background processes
2424
cleanup_() { kill $pid 2>/dev/null && wait $pid; }
2525

2626
SORT_FAILURE=2
2727

28-
seq -w 2000 > exp || fail=1
29-
tac exp > in || fail=1
30-
insize=$(stat -c %s - <in) || fail=1
28+
seq -w 2000 > exp || framework_failure_
29+
tac exp > in || framework_failure_
30+
insize=$(stat -c %s - <in) || framework_failure_
3131

3232
# This compressor's behavior is adjustable via environment variables.
3333
export PRE_COMPRESS=
@@ -42,6 +42,24 @@ EOF
4242

4343
chmod +x compress
4444

45+
# "Early exit" test
46+
#
47+
# In this test, the compressor exits before reading all (any) data.
48+
# Until coreutils 9.9 'sort' could get a SIGPIPE writing to the
49+
# exited processes and silently exit. Note the same issue can happen
50+
# irrespective of exit status. It's more likely to happen in the
51+
# case of the child exiting with success, and if we write more data
52+
# (hence the --batch-size=30 and double "in"). Note we check sort doesn't
53+
# get SIGPIPE rather than if it returns SORT_FAILURE, because there is
54+
# the theoretical possibility that the kernel could buffer the
55+
# amount of data we're writing here and not issue the EPIPE to sort.
56+
# In other words we currently may not detect failures in the extreme edge case
57+
# of writing a small amount of data to a compressor that exits 0
58+
# while not reading all the data presented.
59+
PRE_COMPRESS='exit 0' \
60+
sort --compress-program=./compress -S 1k --batch-size=30 ./in ./in > out
61+
test $(env kill -l $?) = 'PIPE' && fail=1
62+
4563
# "Impatient exit" tests
4664
#
4765
# In these test cases, the biggest compressor (or decompressor) exits

0 commit comments

Comments
 (0)