From f42077036b6b70c93b6d10f5ef5e9334772a843d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Sun, 5 Mar 2023 15:51:32 +0000 Subject: [PATCH] tee: support non blocking outputs Non blocking outputs can be seen for example when piping telnet through tee to a terminal. In that case telnet sets its input to nonblocking mode, which results in tee's output being nonblocking, in which case in may receive an EAGAIN error upon write(). The same issue was seen with mpirun. The following can be used to reproduce this locally at a terminal (in most invocations): $ { dd iflag=nonblock count=0 status=none; dd bs=10K count=10 if=/dev/zero status=none; } | tee || echo fail >/dev/tty * src/iopoll.c (iopoll_internal): A new function refactored from iopoll(), to also support a mode where we check the output descriptor is writeable. (iopoll): Now refactored to just call iopoll_internal(). (fwait_for_nonblocking_write): A new internal function which uses iopoll_internal() to wait for writeable output if an EAGAIN or EWOULDBLOCK was received. (fwrite_nonblock): An fwrite() wrapper which uses fwait_for_nonblocking_write() to handle EAGAIN. (fclose_nonblock): Likewise. src/iopoll.h: Add fclose_nonblock, fwrite_nonblock. src/tee.c: Call fclose_nonblock() and fwrite_nonblock wrappers, instead of the standard functions. * tests/misc/tee.sh: Add a test case. * NEWS: Mention the improvement. The idea was suggested by Kamil Dudka in https://bugzilla.redhat.com/1615467 --- NEWS | 4 ++ src/iopoll.c | 135 ++++++++++++++++++++++++++++++++++++++-------- src/iopoll.h | 3 ++ src/tee.c | 5 +- tests/misc/tee.sh | 17 +++++- 5 files changed, 139 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index 5b0dc939c..3a6b0840c 100644 --- a/NEWS +++ b/NEWS @@ -173,6 +173,10 @@ GNU coreutils NEWS -*- outline -*- tee -p detects when all remaining outputs have become broken pipes, and exits, rather than waiting for more input to induce an exit when written. + tee now handles non blocking outputs, which can be seen for example with + telnet or mpirun piping through tee to a terminal. + Previously tee could truncate data written to such an output and fail, + and also potentially output a "Resource temporarily unavailable" error. * Noteworthy changes in release 9.1 (2022-04-15) [stable] diff --git a/src/iopoll.c b/src/iopoll.c index ca27595e3..b211dafc8 100644 --- a/src/iopoll.c +++ b/src/iopoll.c @@ -1,5 +1,5 @@ -/* iopoll.c -- broken pipe detection (while waiting for input) - Copyright (C) 1989-2022 Free Software Foundation, Inc. +/* iopoll.c -- broken pipe detection / non blocking output handling + Copyright (C) 2022 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -20,10 +20,10 @@ #include - /* poll(2) is needed on AIX (where 'select' gives a readable - event immediately) and Solaris (where 'select' never gave - a readable event). Also use poll(2) on systems we know work - and/or are already using poll (linux). */ +/* poll(2) is needed on AIX (where 'select' gives a readable + event immediately) and Solaris (where 'select' never gave + a readable event). Also use poll(2) on systems we know work + and/or are already using poll (linux). */ #if defined _AIX || defined __sun || defined __APPLE__ || \ defined __linux__ || defined __ANDROID__ @@ -47,25 +47,36 @@ #include "isapipe.h" -/* Wait for FDIN to become ready for reading or FDOUT to become a broken pipe. +/* BROKEN_OUTPUT selects the mode of operation of this function. + If BROKEN_OUTPUT, wait for FDIN to become ready for reading + or FDOUT to become a broken pipe. + If !BROKEN_OUTPUT, wait for FDIN or FDOUT to become ready for writing. If either of those are -1, then they're not checked. Set BLOCK to true to wait for an event, otherwise return the status immediately. Return 0 if not BLOCKing and there is no event on the requested descriptors. Return 0 if FDIN can be read() without blocking, or IOPOLL_BROKEN_OUTPUT if - FDOUT becomes a broken pipe, otherwise IOPOLL_ERROR if there is a poll() - or select() error. */ + FDOUT becomes a broken pipe. If !BROKEN_OUTPUT return 0 if FDOUT writeable. + Otherwise return IOPOLL_ERROR if there is a poll() or select() error. */ -extern int -iopoll (int fdin, int fdout, bool block) +static int +iopoll_internal (int fdin, int fdout, bool block, bool broken_output) { -#if IOPOLL_USES_POLL + assert (fdin != -1 || fdout != -1); +#if IOPOLL_USES_POLL struct pollfd pfds[2] = { /* POLLRDBAND needed for illumos, macOS. */ { .fd = fdin, .events = POLLIN | POLLRDBAND, .revents = 0 }, { .fd = fdout, .events = POLLRDBAND, .revents = 0 }, }; + int check_out_events = POLLERR | POLLHUP | POLLNVAL; int ret = 0; + if (! broken_output) + { + pfds[0].events = pfds[1].events = POLLOUT; + check_out_events = POLLOUT; + } + while (0 <= ret || errno == EINTR) { ret = poll (pfds, 2, block ? -1 : 0); @@ -77,8 +88,8 @@ iopoll (int fdin, int fdout, bool block) assert (0 < ret); if (pfds[0].revents) /* input available or pipe closed indicating EOF; */ return 0; /* should now be able to read() without blocking */ - if (pfds[1].revents) /* POLLERR, POLLHUP (or POLLNVAL) */ - return IOPOLL_BROKEN_OUTPUT; /* output error or broken pipe */ + if (pfds[1].revents & check_out_events) + return broken_output ? IOPOLL_BROKEN_OUTPUT : 0; } #else /* fall back to select()-based implementation */ @@ -96,31 +107,40 @@ iopoll (int fdin, int fdout, bool block) as ready for reading. Assumes fdout is not actually readable. */ while (0 <= ret || errno == EINTR) { - fd_set rfds; - FD_ZERO (&rfds); + fd_set fds; + FD_ZERO (&fds); if (0 <= fdin) - FD_SET (fdin, &rfds); + FD_SET (fdin, &fds); if (0 <= fdout) - FD_SET (fdout, &rfds); + FD_SET (fdout, &fds); struct timeval delay = { .tv_sec = 0, .tv_usec = 0 }; - ret = select (nfds, &rfds, NULL, NULL, block ? NULL : &delay); + ret = select (nfds, + broken_output ? &fds : NULL, + broken_output ? NULL : &fds, + NULL, block ? NULL : &delay); if (ret < 0) continue; if (ret == 0 && ! block) return 0; assert (0 < ret); - if (0 <= fdin && FD_ISSET (fdin, &rfds)) /* input available or EOF; */ + if (0 <= fdin && FD_ISSET (fdin, &fds)) /* input available or EOF; */ return 0; /* should now be able to read() without blocking */ - if (0 <= fdout && FD_ISSET (fdout, &rfds)) /* equiv to POLLERR */ - return IOPOLL_BROKEN_OUTPUT; /* output error or broken pipe */ + if (0 <= fdout && FD_ISSET (fdout, &fds)) /* equiv to POLLERR */ + return broken_output ? IOPOLL_BROKEN_OUTPUT : 0; } #endif return IOPOLL_ERROR; } +extern int +iopoll (int fdin, int fdout, bool block) +{ + return iopoll_internal (fdin, fdout, block, true); +} + /* Return true if fdin is relevant for iopoll(). @@ -145,3 +165,74 @@ iopoll_output_ok (int fdout) { return isapipe (fdout) > 0; } + +#ifdef EWOULDBLOCK +# define IS_EAGAIN(errcode) ((errcode) == EAGAIN || (errcode) == EWOULDBLOCK) +#else +# define IS_EAGAIN(errcode) ((errcode) == EAGAIN) +#endif + +/* Inspect the errno of the previous syscall. + On EAGAIN, wait for the underlying file descriptor to become writable. + Return true, if EAGAIN has been successfully handled. */ + +static bool +fwait_for_nonblocking_write (FILE *f) +{ + if (! IS_EAGAIN (errno)) + /* non-recoverable write error */ + return false; + + int fd = fileno (f); + if (fd == -1) + goto fail; + + /* wait for the file descriptor to become writable */ + if (iopoll_internal (-1, fd, true, false) != 0) + goto fail; + + /* successfully waited for the descriptor to become writable */ + clearerr (f); + return true; + +fail: + errno = EAGAIN; + return false; +} + + +/* wrapper for fclose() that also waits for F if non blocking. */ + +extern bool +fclose_nonblock (FILE *f) +{ + for (;;) + { + if (fclose (f) == 0) + return true; + + if (! fwait_for_nonblocking_write (f)) + return false; + } +} + + +/* wrapper for fwrite() that also waits for F if non blocking. */ + +extern bool +fwrite_nonblock (char const *buf, ssize_t size, FILE *f) +{ + for (;;) + { + const size_t written = fwrite (buf, 1, size, f); + size -= written; + assert (size >= 0); + if (size <= 0) /* everything written */ + return true; + + if (! fwait_for_nonblocking_write (f)) + return false; + + buf += written; + } +} diff --git a/src/iopoll.h b/src/iopoll.h index 00fc99836..79d5ccfef 100644 --- a/src/iopoll.h +++ b/src/iopoll.h @@ -4,3 +4,6 @@ int iopoll (int fdin, int fdout, bool block); bool iopoll_input_ok (int fdin); bool iopoll_output_ok (int fdout); + +bool fclose_nonblock (FILE *f); +bool fwrite_nonblock (char const *buf, ssize_t size, FILE *f); diff --git a/src/tee.c b/src/tee.c index 8da68230a..7785942d6 100644 --- a/src/tee.c +++ b/src/tee.c @@ -26,6 +26,7 @@ #include "die.h" #include "error.h" #include "fadvise.h" +#include "iopoll.h" #include "stdio--.h" #include "xbinary-io.h" #include "iopoll.h" @@ -313,7 +314,7 @@ tee_files (int nfiles, char **files, bool pipe_check) Standard output is the first one. */ for (i = 0; i <= nfiles; i++) if (descriptors[i] - && fwrite (buffer, bytes_read, 1, descriptors[i]) != 1) + && ! fwrite_nonblock (buffer, bytes_read, descriptors[i])) { if (fail_output (descriptors, files, i)) ok = false; @@ -331,7 +332,7 @@ tee_files (int nfiles, char **files, bool pipe_check) /* Close the files, but not standard output. */ for (i = 1; i <= nfiles; i++) - if (descriptors[i] && fclose (descriptors[i]) != 0) + if (descriptors[i] && ! fclose_nonblock (descriptors[i])) { error (0, errno, "%s", quotef (files[i])); ok = false; diff --git a/tests/misc/tee.sh b/tests/misc/tee.sh index 63e7524c0..6dbd28d16 100755 --- a/tests/misc/tee.sh +++ b/tests/misc/tee.sh @@ -79,8 +79,23 @@ touch file.ro || framework_failure_ chmod a-w file.ro || framework_failure_ returns_ 1 tee -p /dev/null && wait $pid; } +read_fifo_delayed() { + { sleep .1; timeout 10 dd of=/dev/null status=none; } < fifo +} +read_fifo_delayed & pid=$! +dd count=20 bs=100K if=/dev/zero status=none | +{ + dd count=0 oflag=nonblock status=none + tee || { cleanup_; touch tee.fail; } +} >fifo +test -f tee.fail && fail=1 + +# Ensure tee honors --output-error modes read_fifo() { timeout 10 dd count=1 if=fifo of=/dev/null status=none & } # Determine platform sigpipe exit status -- 2.26.2