Re: stdio buffer flushing on redirection

[Tom Alsberg]

Given the recent storm of applied patches from the list, I was
wondering if anybody had the time to look at this one.  Does it do the
trick?  Is the bug at all reproducible (with the script I quoted)
elsewhere?  If there is anything that should be changed in it, or
inhibiting it from getting in, let me know.

 Thanks,
 -- Tom

On Sun, Dec 30, 2007 at 05:28:21PM +0200, Tom Alsberg wrote:
> Hi there.
>
> A curious issue I had with a shell script writes output to a pipe, 
> turns out (after quite a while of investigation, and noticing that 
> it works on systems where /bin/sh is not bash) to be due to the way 
> bash writes output on redirection.
>
> Some bash builtins (e.g. echo) write output using stdio functions 
> (e.g. printf, putchar), however redirection is performed at the Unix 
> file descriptor level (using dup).  The thing is that since stdio 
> handles buffering on its own, output of builtins may go to the wrong 
> place if redirection is done.  While bash correctly takes care to 
> flush the buffers after output in e.g. the echo builtin, it does not 
> handle the case of output failure.
>
> In case fflush fails, under some implementations, the output may 
> remain in the stdout buffer, and when printing something later, 
> after redirecting stdout to some file, the previous output (which 
> should not have gone to this file) will remain in the buffer, and be 
> printed together with the new output into the target file.
>
> I managed to reduce the bug to a simple example - take the following 
> script:
>
> #!/usr/bin/env bash
> trap "" 1
> while true; do
>        echo "FOO"
>        echo "BAR" >> /tmp/barlog
> done
>
> and run it within an xterm.  As long as the terminal is still open, 
> all FOO output goes to the terminal, while all BAR output goes to 
> /tmp/barlog.  Close the xterm, though, and (at least with glibc 2.7 
> as the system C library), the FOO output will fail to go to the 
> terminal (which no longer exists), and will instead also go to 
> /tmp/barlog.
>
> I believe if the terminal was closed, one would expect output to it 
> to be discarded, and not go to some other unrelated file, where it 
> may interfere.  If somebody can check this and let me know if you 
> can reproduce the problem, or confirm that this is indeed the 
> problem...
>
> It does not appear easy to fix the bug in a portable way, since in 
> general mixing I/O in the Unix file descriptor level and in the 
> stdio level is not so well defined.  However, 4.4BSD-based systems 
> provide the fpurge function, and Solaris and glibc the (identical, 
> except for the return value) __fpurge function, which clears all 
> pending output in a stdio FILE * buffer.  Calling those functions 
> when redirecting stdout/stderr solves the problem on systems where 
> they are supported - output that previously failed to go to stdout 
> will be discarded and not go to the redirect target. 
>
> Attached is a small patch I wrote for bash-3.2 that (with a small 
> change to configure.in and config.h.in) checks for those functions 
> on configure, and if they are available uses them whenever 
> redirecting stdout/stderr.  That seems to do the trick here, but let 
> me know if it works for you, or if there is a more correct way to do 
> this.
>
> I would be glad to have that patch applied to upstream bash - that 
> will save us the trouble of applying it locally every time.  Of 
> course it would be even better to solve that problem in a more 
> portable manner, but I suspect that may require more substantial 
> modifications to parts of the code.
>
>  Cheers,
>  -- Tom
>
> --
>  Tom Alsberg - hacker (being the best description fitting this space)
>  Web page:      http://www.cs.huji.ac.il/~alsbergt/
> DISCLAIMER:  The above message does not even necessarily represent what
> my fingers have typed on the keyboard, save anything further.

> diff -ur bash-3.2.orig/config.h.in bash-3.2/config.h.in
> --- bash-3.2.orig/config.h.in   2006-09-12 23:00:54.000000000 +0300
> +++ bash-3.2/config.h.in        2007-12-30 15:41:05.974625000 +0200
> @@ -812,6 +812,13 @@
>  /* Define if you have the wcwidth function.  */
>  #undef HAVE_WCWIDTH
>
> +/* Define if you have the fpurge function.  */
> +#undef HAVE_FPURGE
> +
> +/* Define if you have the __fpurge function.  */
> +#undef HAVE___FPURGE
> +
> +
>  /* Presence of certain system include files. */
>
>  /* Define if you have the <arpa/inet.h> header file. */
> diff -ur bash-3.2.orig/configure.in bash-3.2/configure.in
> --- bash-3.2.orig/configure.in  2006-09-26 18:05:45.000000000 +0300
> +++ bash-3.2/configure.in       2007-12-30 15:14:52.206078000 +0200
> @@ -721,6 +721,9 @@
>  AC_CHECK_DECLS([strcpy])
>  AC_CHECK_DECLS([strsignal])
>
> +dnl checks for fpurge or __fpurge
> +AC_CHECK_FUNCS(fpurge __fpurge)
> +
>  dnl Extra test to detect the horribly broken HP/UX 11.00 strtold(3)
>  AC_CHECK_DECLS([strtold], [
>      AC_MSG_CHECKING([for broken strtold])
> diff -ur bash-3.2.orig/redir.c bash-3.2/redir.c
> --- bash-3.2.orig/redir.c       2006-05-24 05:31:35.000000000 +0300
> +++ bash-3.2/redir.c    2007-12-30 16:35:53.627539000 +0200
> @@ -31,6 +31,18 @@
>  #include "filecntl.h"
>  #include "posixstat.h"
>
> +
> +/* check for a way to clear the output buffer of a FILE *, for use
> +   after dup on stdout/stderr */
> +#undef FPURGE
> +#ifdef HAVE___FPURGE
> +#define        FPURGE  __fpurge
> +#endif
> +#ifdef HAVE_FPURGE
> +#define        FPURGE  fpurge
> +#endif
> +
> +
>  #if defined (HAVE_UNISTD_H)
>  #  include <unistd.h>
>  #endif
> @@ -769,6 +781,19 @@
>           if ((fd != redirector) && (dup2 (fd, redirector) < 0))
>             return (errno);
>
> +
> +/* if possible, and the redirecting stdout/stderr, clear the stdio
> +   output buffers, so output from shell builtins that previously
> +   failed will not go to the redirected file */
> +#ifdef FPURGE
> +             if (redirector == 1) {
> +                 FPURGE(stdout);
> +             } else if (redirector == 2) {
> +                 FPURGE(stderr);
> +             }
> +#endif
> +
> +
>  #if defined (BUFFERED_INPUT)
>           /* Do not change the buffered stream for an implicit redirection
>              of /dev/null to fd 0 for asynchronous commands without job
> @@ -851,6 +876,19 @@
>                   return (r);
>                 }
>
> +
> +/* if possible, and the redirecting stdout/stderr, clear the stdio
> +   output buffers, so output from shell builtins that previously
> +   failed will not go to the redirected file */
> +#ifdef FPURGE
> +             if (redirector == 1) {
> +                 FPURGE(stdout);
> +             } else if (redirector == 2) {
> +                 FPURGE(stderr);
> +             }
> +#endif
> +
> +
>  #if defined (BUFFERED_INPUT)
>               duplicate_buffered_stream (fd, redirector);
>  #endif
> @@ -890,6 +928,19 @@
>           if (dup2 (redir_fd, redirector) < 0)
>             return (errno);
>
> +
> +/* if possible, and the redirecting stdout/stderr, clear the stdio
> +   output buffers, so output from shell builtins that previously
> +   failed will not go to the redirected file */
> +#ifdef FPURGE
> +         if (redirector == 1) {
> +             FPURGE(stdout);
> +         } else if (redirector == 2) {
> +             FPURGE(stderr);
> +         }
> +#endif
> +
> +
>  #if defined (BUFFERED_INPUT)
>           if (ri == r_duplicating_input || ri == r_move_input)
>             duplicate_buffered_stream (redir_fd, redirector);

--
 Tom Alsberg - hacker (being the best description fitting this space)
 Web page:      http://www.cs.huji.ac.il/~alsbergt/
DISCLAIMER:  The above message does not even necessarily represent what
my fingers have typed on the keyboard, save anything further.