Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Aaak! Sorry, please disregard the patchset in my previous email. I messed up a rebase I was doing. I am attaching a corrected patchset now. Thanks! Carl On Thu, 15 Dec 2022, Carl Edquist wrote: Hi Pádraig, At your convenience, please see my attached patch set. ... A couple very small implementation details I guess I'll mention here. 1. In iopoll.c, I use isapipe() from gnulib, as is done in tail.c. I am not wild about the gnulib implementation of isapipe() - I would rather do a simple S_ISFIFO check, as the FIFO vs pipe distinction is irrelevant for iopoll(), and I'm not sure whether the "pipes are sockets" for Darwin 7.7 still needs to be supported. But I was happy to see (at least for me on Linux) it looks like the config.h macros end up compiling away the complicated parts of this function, so I guess it's not that bad :) 2. I believe the "if (errno == EINTR) continue;" logic in iopoll() is unnecessary, as an interrupted poll() or select() call will result in repeating the loop anyway. I've left it for clarity, but as far as I can tell it could be removed just the same. On Tue, 13 Dec 2022, Arsen Arsenović wrote: It might also be good to give a quick test on FreeBSD, since it has some popularity too. I don't have easy access to play with this, but yeah I think I've seen the BSD man page for poll(2), and I imagine poll would work there. If so we can tweak the IOPOLL_USES_POLL definition in iopoll.c to include an appropriate preprocessor check for BSD. If you prefer, I'll have some time in the latter part of this week too. Let's not forget to include the testcase posted previously (with -p instead of -P, since it was suggested to enable polling for -p): Hi Arsen, Would you like to add your test case and comments in a separate commit? Thanks! Carl ( sleep 5 | (timeout 3 tee -p 2>/dev/null && echo TEST_PASSED >&8) | : ) 8>&1 | grep -qx TEST_PASSED To annotate it, and let's include this info in a comment: - sleep emulates misbehaving input. - The timeout is our failure safety-net. - We ignore stderr from tee, and should have no stdout anyway. - If that succeeds, we print TEST_PASSED into FD 8 to grep for later. (FD 8 was selected by a D20 roll, or rather, a software emulation) - The colon is the immediately closed output process. - We redirect 8 back into stdout to grep it. If tee fails, for instance because it times out, or it fails to recognize -P for some reason, the echo simply won't run. The grep options are in POSIX (or, at least, in POSIX.1-2017). Thank you both, have a great night. -- Arsen Arsenović From 04689f0483a0ef1f27ab16fe521191d5e7cf1112 Mon Sep 17 00:00:00 2001 From: Carl Edquist Date: Thu, 15 Dec 2022 06:10:33 -0600 Subject: [PATCH 1/2] iopoll: broken pipe detection while waiting for input When a program's output becomes a broken pipe, future attempts to write to that ouput will fail (SIGPIPE/EPIPE). Once it is known that all future write attepts will fail (due to broken pipes), in many cases it becomes pointless to wait for further input for slow devices like ttys. Ideally, a program could use this information to exit early once it is known that future writes will fail. Introduce iopoll() to wait on a pair of fds (input & output) for input to become ready or output to become a broken pipe. This is relevant when input is intermittent (a tty, pipe, or socket); but if input is always ready (a regular file or block device), then a read() will not block, and write failures for a broken pipe will happen normally. Introduce iopoll_input_ok() to check whether an input fd is relevant for iopoll(). Experimentally, broken pipes are only detectable immediately for pipes, but not sockets. Errors for other file types will be detected in the usual way, on write failure. Introduce iopoll_output_ok() to check whether an output fd is suitable for iopoll() -- namely, whether it is a pipe. iopoll() is best implemented with a native poll(2) where possible, but fall back to a select(2)-based implementation platforms where there are portability issues. See also discussion in tail.c. In general, adding a call to iopoll() before a read() in filter programs also allows broken pipes to "propagate" backwards in a shell pipeline. * src/iopoll.c, src/iopoll.h (iopoll): New function implementing broken pipe detection on output while waiting for input. (IOPOLL_BROKEN_OUTPUT, IOPOLL_ERROR): Return codes for iopoll(). (IOPOLL_USES_POLL): Macro for poll() vs select() implementation. (iopoll_input_ok): New function to check whether an input fd is relevant for iopoll(). (iopoll_output_ok): New function to check whether an input fd is suitable for iopoll(). * src/local.mk (noinst_HEADERS): add src/iopoll.h. --- src/iopoll.c | 115 +++ src/iopoll.h | 6 src/local.mk | 1 + 3 files changed, 122 insertions(+) create mode 100644 src/iopoll.c create mode 100644 src/iopoll.h
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Hi Pádraig, O2 At your convenience, please see my attached patch set. ... A couple very small implementation details I guess I'll mention here. 1. In iopoll.c, I use isapipe() from gnulib, as is done in tail.c. I am not wild about the gnulib implementation of isapipe() - I would rather do a simple S_ISFIFO check, as the FIFO vs pipe distinction is irrelevant for iopoll(), and I'm not sure whether the "pipes are sockets" for Darwin 7.7 still needs to be supported. But I was happy to see (at least for me on Linux) it looks like the config.h macros end up compiling away the complicated parts of this function, so I guess it's not that bad :) 2. I believe the "if (errno == EINTR) continue;" logic in iopoll() is unnecessary, as an interrupted poll() or select() call will result in repeating the loop anyway. I've left it for clarity, but as far as I can tell it could be removed just the same. On Tue, 13 Dec 2022, Arsen Arsenović wrote: It might also be good to give a quick test on FreeBSD, since it has some popularity too. I don't have easy access to play with this, but yeah I think I've seen the BSD man page for poll(2), and I imagine poll would work there. If so we can tweak the IOPOLL_USES_POLL definition in iopoll.c to include an appropriate preprocessor check for BSD. If you prefer, I'll have some time in the latter part of this week too. Let's not forget to include the testcase posted previously (with -p instead of -P, since it was suggested to enable polling for -p): Hi Arsen, Would you like to add your test case and comments in a separate commit? Thanks! Carl ( sleep 5 | (timeout 3 tee -p 2>/dev/null && echo TEST_PASSED >&8) | : ) 8>&1 | grep -qx TEST_PASSED To annotate it, and let's include this info in a comment: - sleep emulates misbehaving input. - The timeout is our failure safety-net. - We ignore stderr from tee, and should have no stdout anyway. - If that succeeds, we print TEST_PASSED into FD 8 to grep for later. (FD 8 was selected by a D20 roll, or rather, a software emulation) - The colon is the immediately closed output process. - We redirect 8 back into stdout to grep it. If tee fails, for instance because it times out, or it fails to recognize -P for some reason, the echo simply won't run. The grep options are in POSIX (or, at least, in POSIX.1-2017). Thank you both, have a great night. -- Arsen Arsenović From d2a62057cacd25730243c20506edc66a27e3bf0d Mon Sep 17 00:00:00 2001 From: Carl Edquist Date: Thu, 15 Dec 2022 06:10:33 -0600 Subject: [PATCH 1/2] iopoll: broken pipe detection while waiting for input When a program's output becomes a broken pipe, future attempts to write to that ouput will fail (SIGPIPE/EPIPE). Once it is known that all future write attepts will fail (due to broken pipes), in many cases it becomes pointless to wait for further input for slow devices like ttys. Ideally, a program could use this information to exit early once it is known that future writes will fail. Introduce iopoll() to wait on a pair of fds (input & output) for input to become ready or output to become a broken pipe. This is relevant when input is intermittent (a tty, pipe, or socket); but if input is always ready (a regular file or block device), then a read() will not block, and write failures for a broken pipe will happen normally. Introduce iopoll_input_ok() to check whether an input fd is relevant for iopoll(). Experimentally, broken pipes are only detectable immediately for pipes, but not sockets. Errors for other file types will be detected in the usual way, on write failure. Introduce iopoll_output_ok() to check whether an output fd is suitable for iopoll() -- namely, whether it is a pipe. iopoll() is best implemented with a native poll(2) where possible, but fall back to a select(2)-based implementation platforms where there are portability issues. See also discussion in tail.c. In general, adding a call to iopoll() before a read() in filter programs also allows broken pipes to "propagate" backwards in a shell pipeline. * src/iopoll.c, src/iopoll.h (iopoll): New function implementing broken pipe detection on output while waiting for input. (IOPOLL_BROKEN_OUTPUT, IOPOLL_ERROR): Return codes for iopoll(). (IOPOLL_USES_POLL): Macro for poll() vs select() implementation. (iopoll_input_ok): New function to check whether an input fd is relevant for iopoll(). (iopoll_output_ok): New function to check whether an input fd is suitable for iopoll(). * src/local.mk (noinst_HEADERS): add src/iopoll.h. --- src/iopoll.c | 128 +++ src/iopoll.h | 6 +++ src/local.mk | 1 + 3 files changed, 135 insertions(+) create mode 100644 src/iopoll.c create mode 100644 src/iopoll.h diff --git a/src/iopoll.c b/src/iopoll.c new file mode 100644 index 000..32462d8 --- /dev/null +++ b/src/iopoll.c @@ -0,0 +1,128 @@ +/* iopoll.c -- broken pipe detection (while waiting for input) + Copyright (C) 1
