Re: [PATCH] Re: SIGPIPE and tee
Thanks - that change is great. Busybox tee now propagates SIGPIPE just like
GNU tee does by default.
Sam
On Mon, 7 Oct 2019 at 13:27, Denys Vlasenko
wrote:
> On Fri, Oct 4, 2019 at 11:20 AM Sam Liddicott wrote:
> > nudge on this attached patch
> >
> > -- Forwarded message -
> > From: Sam Liddicott
> > Date: Fri, 13 Sep 2019 at 11:15
> > Subject: Re: SIGPIPE and tee
> > To: busybox
> >
> >
> > Here is a patch which invokes kill_myself_with_sig(SIGPIPE) if fwrite
> fails with errno=EPIPE.
> >
> > Quitting with a signal rather than a diagnostic output emulates GNU tee
> which is in line with the apparent reason for busybox tee in ignoring
> SIGPIPE in the first place.
>
>
> >
> > I have tested it with and without CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
> >
> > As expected, dd succeeds for bs=1024 with
> CONFIG_FEATURE_TEE_USE_BLOCK_IO=y but fails for without.
> > dd fails in both cases for bs=1024000
> > tee fails in both cases
> >
> > ( sleep 2 ; dd if=/dev/zero bs=1024000 count=5 ; echo dd $? > /dev/tty ;
> ) | {
> build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
> tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
> >
> > ( sleep 2 ; dd if=/dev/zero bs=1024 count=5 ; echo dd $? > /dev/tty ; )
> | {
> build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
> tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
> >
> > A colleague has suggested that any write failure to stdout by tee ought
> to cause tee to quit, but I leave this to your consideration; something
> like this might cover both cases but I haven't tested it:
> >
> > if (fwrite(buf, 1, c, *fp) != c) {
> > if (errno == EPIPE) kill_myself_with_sig(SIGPIPE);
> > retval = EXIT_FAILURE;
> > break;
> > }
> >
> > Sam
> >
> >
> > On Thu, 12 Sep 2019 at 20:03, Sam Liddicott wrote:
> >>
> >> In https://git.busybox.net/busybox/tree/coreutils/tee.c we read:
> >>
> >> /* gnu tee ignores SIGPIPE in case one of the output files is a pipe
> >> * that doesn't consume all its input. Good idea... */
> >> signal(SIGPIPE, SIG_IGN);
> >>
> >>
> >> Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting
> when stdout (as a pipe) is closed.
> >>
> >> Despite the comment, GNU tee does not behave as the comment suggests.
>
> Exactly. GNU tee does not intercept SIGPIPE (unless -p is given).
> I'm removing SIGPIPE handling.
> Please try current git.
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Re: SIGPIPE and tee
On Fri, Oct 4, 2019 at 11:20 AM Sam Liddicott wrote:
> nudge on this attached patch
>
> -- Forwarded message -
> From: Sam Liddicott
> Date: Fri, 13 Sep 2019 at 11:15
> Subject: Re: SIGPIPE and tee
> To: busybox
>
>
> Here is a patch which invokes kill_myself_with_sig(SIGPIPE) if fwrite fails
> with errno=EPIPE.
>
> Quitting with a signal rather than a diagnostic output emulates GNU tee which
> is in line with the apparent reason for busybox tee in ignoring SIGPIPE in
> the first place.
>
> I have tested it with and without CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
>
> As expected, dd succeeds for bs=1024 with CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
> but fails for without.
> dd fails in both cases for bs=1024000
> tee fails in both cases
>
> ( sleep 2 ; dd if=/dev/zero bs=1024000 count=5 ; echo dd $? > /dev/tty ; ) |
> { build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
> tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
>
> ( sleep 2 ; dd if=/dev/zero bs=1024 count=5 ; echo dd $? > /dev/tty ; ) | {
> build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid tee
> /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
>
> A colleague has suggested that any write failure to stdout by tee ought to
> cause tee to quit, but I leave this to your consideration; something like
> this might cover both cases but I haven't tested it:
>
> if (fwrite(buf, 1, c, *fp) != c) {
> if (errno == EPIPE) kill_myself_with_sig(SIGPIPE);
> retval = EXIT_FAILURE;
> break;
> }
>
> Sam
>
>
> On Thu, 12 Sep 2019 at 20:03, Sam Liddicott wrote:
>>
>> In https://git.busybox.net/busybox/tree/coreutils/tee.c we read:
>>
>> /* gnu tee ignores SIGPIPE in case one of the output files is a pipe
>> * that doesn't consume all its input. Good idea... */
>> signal(SIGPIPE, SIG_IGN);
>>
>>
>> Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when
>> stdout (as a pipe) is closed.
>>
>> Despite the comment, GNU tee does not behave as the comment suggests.
Exactly. GNU tee does not intercept SIGPIPE (unless -p is given).
I'm removing SIGPIPE handling.
Please try current git.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] Re: SIGPIPE and tee
nudge on this attached patch
-- Forwarded message -
From: Sam Liddicott
Date: Fri, 13 Sep 2019 at 11:15
Subject: Re: SIGPIPE and tee
To: busybox
Here is a patch which invokes kill_myself_with_sig(SIGPIPE) if fwrite fails
with errno=EPIPE.
Quitting with a signal rather than a diagnostic output emulates GNU tee
which is in line with the apparent reason for busybox tee in ignoring
SIGPIPE in the first place.
I have tested it with and without CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
As expected, dd succeeds for bs=1024 with CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
but fails for without.
dd fails in both cases for bs=1024000
tee fails in both cases
( sleep 2 ; dd if=/dev/zero bs=1024000 count=5 ; echo dd $? > /dev/tty ; )
| {
build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
( sleep 2 ; dd if=/dev/zero bs=1024 count=5 ; echo dd $? > /dev/tty ; ) | {
build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
A colleague has suggested that *any* write failure to *stdout* by tee ought
to cause tee to quit, but I leave this to your consideration; something
like this *might* cover both cases but I haven't tested it:
if (fwrite(buf, 1, c, *fp) != c) {
if (errno == EPIPE) kill_myself_with_sig(SIGPIPE);
retval = EXIT_FAILURE;
break;
}
Sam
On Thu, 12 Sep 2019 at 20:03, Sam Liddicott wrote:
> In https://git.busybox.net/busybox/tree/coreutils/tee.c we read:
>
> /* gnu tee ignores SIGPIPE in case one of the output files is a pipe
> * that doesn't consume all its input. Good idea... */
> signal(SIGPIPE, SIG_IGN);
>
>
> Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when
> stdout (as a pipe) is closed.
>
> Despite the comment, GNU tee does not behave as the comment suggests.
>
> Comparing 1.27.2-2ubuntu3.2 with tee (GNU coreutils) 8.28
>
> I test with one output file and stdout that stops early causing SIGPIPE
>
> 1. GNU tee
>
> ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( tee
> /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null
>
> 10+0 records in
> 10+0 records out
> 100 bytes copied, 0.000387276 s, 258 kB/s
> tee 141
> done 141
>
> the first two pipeline stages have exit code 141 due to sigpipe as we
> would expect
>
> 2. busybox tee
>
> ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( busybox tee
> /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null
> 10+0 records in
> 10+0 records out
> 100 bytes copied, 0.000291134 s, 343 kB/s
> 8192+0 records in
> 8192+0 records out
> 8388608 bytes (8.4 MB, 8.0 MiB) copied, 0.0447136 s, 188 MB/s
> done 0
> tee 0
>
> both prior pipeline elements have exit code zero and the first stage
> processed all of the data
>
> The POSIX spec for tee reads:
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html
> CONSEQUENCES OF ERRORS
>
> If a write to any successfully opened *file* operand fails, writes to
> other successfully opened *file* operands and standard output shall
> continue, but the exit status shall be non-zero. Otherwise, the default
> actions specified in *Utility Description Defaults*
> <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap01.html#tag_17_04>
> apply.
>
> But this case deals with a failure to write to stdout, not a failure to
> write to a *file* operand.
>
> Certainly, with this behaviour, busybox tee does not comply with GNU tee
> as it appears to intend, so I suggest that the line signal(SIGPIPE,
> SIG_IGN) be removed.
>
> To meet the probable intent, the signal should probably be ignored while
> writing to file operands (in case they turn out to be pipes) and enabled
> each time when writing to stdout.
>
> Sam
>
If we get EPIPE on write to stdout, then fail like a decent tee.
We fail with SIGPIPE like GNU tee as emulating GNU tee seems to be the aim.
--- ./coreutils/tee.c.o 2019-09-13 07:50:47.134804213 +0100
+++ ./coreutils/tee.c 2019-09-13 09:09:40.280881938 +0100
@@ -70,7 +70,9 @@
}
retval = EXIT_SUCCESS;
/* gnu tee ignores SIGPIPE in case one of the output files is a pipe
- * that doesn't consume all its input. Good idea... */
+ * that doesn't consume all its input. Good idea...
+ * An even better idea is to still respect SIGPIPE on stdout
+ * which we do further on. */
signal(SIGPIPE, SIG_IGN);
/* Allocate an array of FILE *'s, with one extra for a sentinel. */
@@ -100,9 +102,12 @@
#if ENABLE_FEATURE_TEE_USE_BLOCK_IO
while ((c = safe_read(STDIN_FILENO, buf, COMMON_BUFSIZE)) > 0) {
fp = files;
- do
+ if (fwrite(buf, 1, c, *fp) != c && errno == EPIPE) {
+ kill_myself_with_s
Re: SIGPIPE and tee
Here is a patch which invokes kill_myself_with_sig(SIGPIPE) if fwrite fails
with errno=EPIPE.
Quitting with a signal rather than a diagnostic output emulates GNU tee
which is in line with the apparent reason for busybox tee in ignoring
SIGPIPE in the first place.
I have tested it with and without CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
As expected, dd succeeds for bs=1024 with CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
but fails for without.
dd fails in both cases for bs=1024000
tee fails in both cases
( sleep 2 ; dd if=/dev/zero bs=1024000 count=5 ; echo dd $? > /dev/tty ; )
| {
build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
( sleep 2 ; dd if=/dev/zero bs=1024 count=5 ; echo dd $? > /dev/tty ; ) | {
build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
A colleague has suggested that *any* write failure to *stdout* by tee ought
to cause tee to quit, but I leave this to your consideration; something
like this *might* cover both cases but I haven't tested it:
if (fwrite(buf, 1, c, *fp) != c) {
if (errno == EPIPE) kill_myself_with_sig(SIGPIPE);
retval = EXIT_FAILURE;
break;
}
Sam
On Thu, 12 Sep 2019 at 20:03, Sam Liddicott wrote:
> In https://git.busybox.net/busybox/tree/coreutils/tee.c we read:
>
> /* gnu tee ignores SIGPIPE in case one of the output files is a pipe
> * that doesn't consume all its input. Good idea... */
> signal(SIGPIPE, SIG_IGN);
>
>
> Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when
> stdout (as a pipe) is closed.
>
> Despite the comment, GNU tee does not behave as the comment suggests.
>
> Comparing 1.27.2-2ubuntu3.2 with tee (GNU coreutils) 8.28
>
> I test with one output file and stdout that stops early causing SIGPIPE
>
> 1. GNU tee
>
> ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( tee
> /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null
>
> 10+0 records in
> 10+0 records out
> 100 bytes copied, 0.000387276 s, 258 kB/s
> tee 141
> done 141
>
> the first two pipeline stages have exit code 141 due to sigpipe as we
> would expect
>
> 2. busybox tee
>
> ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( busybox tee
> /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null
> 10+0 records in
> 10+0 records out
> 100 bytes copied, 0.000291134 s, 343 kB/s
> 8192+0 records in
> 8192+0 records out
> 8388608 bytes (8.4 MB, 8.0 MiB) copied, 0.0447136 s, 188 MB/s
> done 0
> tee 0
>
> both prior pipeline elements have exit code zero and the first stage
> processed all of the data
>
> The POSIX spec for tee reads:
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html
> CONSEQUENCES OF ERRORS
>
> If a write to any successfully opened *file* operand fails, writes to
> other successfully opened *file* operands and standard output shall
> continue, but the exit status shall be non-zero. Otherwise, the default
> actions specified in *Utility Description Defaults*
> <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap01.html#tag_17_04>
> apply.
>
> But this case deals with a failure to write to stdout, not a failure to
> write to a *file* operand.
>
> Certainly, with this behaviour, busybox tee does not comply with GNU tee
> as it appears to intend, so I suggest that the line signal(SIGPIPE,
> SIG_IGN) be removed.
>
> To meet the probable intent, the signal should probably be ignored while
> writing to file operands (in case they turn out to be pipes) and enabled
> each time when writing to stdout.
>
> Sam
>
0001-Add-SIGPIPE-handler-to-tee.patch
Description: Binary data
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: SIGPIPE and tee
I also note that busybox tee treats a filename single hyphen as stdout, contrary to POSIX. I mention it only because it might save a few bytes to remove this special case, and treat it as a regular file. https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html OPERANDS The following operands shall be supported: *file*A pathname of an output file. If a *file* operand is '-', it shall refer to a file named *-*; implementations shall not treat it as meaning standard output. On Thu, 12 Sep 2019 at 20:03, Sam Liddicott wrote: > In https://git.busybox.net/busybox/tree/coreutils/tee.c we read: > > /* gnu tee ignores SIGPIPE in case one of the output files is a pipe > * that doesn't consume all its input. Good idea... */ > signal(SIGPIPE, SIG_IGN); > > > Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when > stdout (as a pipe) is closed. > > Despite the comment, GNU tee does not behave as the comment suggests. > > Comparing 1.27.2-2ubuntu3.2 with tee (GNU coreutils) 8.28 > > I test with one output file and stdout that stops early causing SIGPIPE > > 1. GNU tee > > ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( tee > /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null > > 10+0 records in > 10+0 records out > 100 bytes copied, 0.000387276 s, 258 kB/s > tee 141 > done 141 > > the first two pipeline stages have exit code 141 due to sigpipe as we > would expect > > 2. busybox tee > > ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( busybox tee > /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null > 10+0 records in > 10+0 records out > 100 bytes copied, 0.000291134 s, 343 kB/s > 8192+0 records in > 8192+0 records out > 8388608 bytes (8.4 MB, 8.0 MiB) copied, 0.0447136 s, 188 MB/s > done 0 > tee 0 > > both prior pipeline elements have exit code zero and the first stage > processed all of the data > > The POSIX spec for tee reads: > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html > CONSEQUENCES OF ERRORS > > If a write to any successfully opened *file* operand fails, writes to > other successfully opened *file* operands and standard output shall > continue, but the exit status shall be non-zero. Otherwise, the default > actions specified in *Utility Description Defaults* > <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap01.html#tag_17_04> > apply. > > But this case deals with a failure to write to stdout, not a failure to > write to a *file* operand. > > Certainly, with this behaviour, busybox tee does not comply with GNU tee > as it appears to intend, so I suggest that the line signal(SIGPIPE, > SIG_IGN) be removed. > > To meet the probable intent, the signal should probably be ignored while > writing to file operands (in case they turn out to be pipes) and enabled > each time when writing to stdout. > > Sam > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
SIGPIPE and tee
In https://git.busybox.net/busybox/tree/coreutils/tee.c we read: /* gnu tee ignores SIGPIPE in case one of the output files is a pipe * that doesn't consume all its input. Good idea... */ signal(SIGPIPE, SIG_IGN); Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when stdout (as a pipe) is closed. Despite the comment, GNU tee does not behave as the comment suggests. Comparing 1.27.2-2ubuntu3.2 with tee (GNU coreutils) 8.28 I test with one output file and stdout that stops early causing SIGPIPE 1. GNU tee ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( tee /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null 10+0 records in 10+0 records out 100 bytes copied, 0.000387276 s, 258 kB/s tee 141 done 141 the first two pipeline stages have exit code 141 due to sigpipe as we would expect 2. busybox tee ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( busybox tee /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null 10+0 records in 10+0 records out 100 bytes copied, 0.000291134 s, 343 kB/s 8192+0 records in 8192+0 records out 8388608 bytes (8.4 MB, 8.0 MiB) copied, 0.0447136 s, 188 MB/s done 0 tee 0 both prior pipeline elements have exit code zero and the first stage processed all of the data The POSIX spec for tee reads: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html CONSEQUENCES OF ERRORS If a write to any successfully opened *file* operand fails, writes to other successfully opened *file* operands and standard output shall continue, but the exit status shall be non-zero. Otherwise, the default actions specified in *Utility Description Defaults* <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap01.html#tag_17_04> apply. But this case deals with a failure to write to stdout, not a failure to write to a *file* operand. Certainly, with this behaviour, busybox tee does not comply with GNU tee as it appears to intend, so I suggest that the line signal(SIGPIPE, SIG_IGN) be removed. To meet the probable intent, the signal should probably be ignored while writing to file operands (in case they turn out to be pipes) and enabled each time when writing to stdout. Sam ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
