From a9d369226fb8ff3c700ed95250ace92e056368f7 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 in nonblocking mode, which results in tee's output being non blocking, 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 with telnet: $ truncate -s64K ~/public_html/big.file # NULs won't spam terminal $ telnet localhost 80 | strace -e write -o tee.strace src/tee # Type: GET /big.file HTTP/1.0 # Look for handled EAGAIN in tee.strace * 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(). (wait_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 wait_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. * NEWS: Mention the improvement. The idea was suggested by Kamil Dudka in https://bugzilla.redhat.com/1615467 --- NEWS | 4 ++ src/iopoll.c | 129 +++++++++++++++++++++++++++++++++++++++++++-------- src/iopoll.h | 3 ++ src/tee.c | 5 +- 4 files changed, 119 insertions(+), 22 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..585405e3b 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,7 +47,9 @@ #include "isapipe.h" -/* Wait for FDIN to become ready for reading or FDOUT to become a broken pipe. +/* 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. @@ -55,17 +57,25 @@ FDOUT becomes a broken pipe, otherwise 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 +87,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 +106,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 +164,73 @@ 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 + +/* On EAGAIN, wait for the underlying file descriptor to become writable. + Return true, if EAGAIN has been successfully handled. */ + +static bool +wait_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 handles EAGAIN iff F is non blocking. */ + +extern bool +fclose_nonblock (FILE *f) +{ + for (;;) + { + if (fclose (f) == 0) + return true; + + if (! wait_for_nonblocking_write (f)) + return false; + } +} + + +/* wrapper for fwrite() that handles EAGAIN iff F is 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 (! wait_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; -- 2.26.2