Re: inconsistent output for cd

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> > Curiously, under the same conditions on a Red Hat Linux box:
> 
> Why didn't you say so in the first place?  If this behavior only occurs on
> cygwin, then it is a bug in either cygwin or the bash port of cygwin, and
> you should have reported it to the cygwin mailing list, rather than
> forcing upstream to come up with the workaround.  At any rate, since I
> maintain the cygwin port of bash, I'll look into it; I can certainly
> reproduce it, so it looks like there is a problem with file redirection on
> builtins inside command substitution on cygwin.
> 
> $ pwd
> /home/eblake/bash
> $ command cd bash
> /home/eblake/bash
> $ command cd bash 2>/dev/null 1>&2
> $ echo $(command cd bash 2>/dev/null 1>&2)
> /home/eblake/bash
> $

I finally found it.  By default, bash changes stdout to line-buffered mode, 
regardless of whether stdout is an interactive device (POSIX states that on 
process startup, stdout is line-buffered if and only if it is directed to an 
interactive device).  Then, in command_substitute, bash changes the underlying 
fd of stdout as part of the pipe creation, but doesn't inform the stdio library 
about this change.  And the 'cd' builtin uses printf.

Out of the box, cd thus inherits whatever settings stdout had prior to the pipe 
creation (ie. stdout is still line-buffered, even though a normal process exec 
with stdout directed to a pipe would be fully buffered).  This IS a performance 
hit (doing output in a line-buffered rather than fully-buffered fashion 
inherently means more syscalls), althought the performance hit is probably in 
the noise.  But try as I might on Linux, I haven't yet come up with a case 
where this causes observable incorrect behavior (but this doesn't mean such a 
case does not exist).

HOWEVER, on cygwin, there IS a case where this is observably wrong.  By 
default, pipes on cygwin are binary.  But if stdout is directed to a text file, 
then builtin commands (such as cd) invoked inside command substitution will 
inherit the setting of stdout, and output spurious carriage returns - in other 
words, invoking cd by itself vs. in a command substitution has different 
results, when stdout was already pointed to a text file.

I fixed this in the cygwin port in January by adding a call to
 freopen(NULL, "w", stdout);
inside subst.c command_substitute(), on the child side of the pipe.  That 
forces stdio to reevaluate the underlying fd, and recognize any swap from text 
to binary mode.

Bash could always use freopen(NULL) after reassigning the underlying fd of any 
of the std streams, to guarantee that state inappropriate to the new fd is not 
being propagated.  However, POSIX states that freopen(NULL) is not very 
portable (a legal implementation is allowed to return EBADF, closing the file 
in the process), so here I feel safer leaving the use of freopn as cygwin-
specific, where we know freopen(NULL) works, and where it addresses an 
observable bug (namely, that pipes must not inherit text mode from the previous 
state of the file).

Unfortunately, the use of freopen ALSO changes whether stdout is line-
buffered.  Since cd.def calls printf, but not fflush, and the output is no 
longer line buffered, the cygwin bash binary was buffering the output up until 
the command substitution child process exited, whereas it used to print the 
output immediately.

In the case of the bug:
 $ echo $(command cd bash >/dev/null)
bash only forks once - it must fork for $(), but once inside, it recognizes 
that both command and cd are builtins, and that it can get away without any 
further forking, even though it does redirection.  Somewhere along the way, the 
original fd gets dup'd back into place, undoing the >/dev/null.  Then, the 
pending output in stdout FINALLY gets flushed as the child process exits, 
leaking the cd output to the shell.

In my experimentation, adding a call to
 sh_setlinebuf(stdout);
after the cygwin-specific freopen in command_substitute() rectifies the 
problem, by letting cd once again always have line-buffered stdout.  But I 
think that is just treating the symptom; there are other lurking upstream bugs 
that should be fixed regardless of whether my cygwin-specific 
freopen/sh_setlinebuf is applied.

1. Builtins should ALWAYS call fflush if they write to stdout, because they 
cannot rely on being invoked in a subprocess where exiting the process will 
implicitly flush the data in time.  Of the builtins that print to stdout, most 
of them end output in '\n', and were thus relying on stdout being line-buffered 
for implicit flushing (and of those that can output without a trailing '\n', 
both echo and printf were already calling fflush).  I was able to fix this 
particular testcase by adding an fflush in builtins/cd.def bindpwd().

2. It is also possible to fix the bug by ensuring that the std streams are in a 
consistent state prior to changing the underlying fd.  Inside of redir.c 
do_redirection_internal(), adding the line
 if (redirector == 1) fflush (stdout);
immediately before the dup2 in the r_duplicating_output case, also makes the 
bug go away.

3. Why does bash insist that stdout always be line-buffered?  What would break 
if bash relied instead on the POSIX default of line-buffered iff attached to an 
interactive device?  I couldn't find anywhere that POSIX required line 
buffering for sh in non-interactive situations.

-- 
Eric Blake
volunteer cygwin bash port maintainer