Re: Possible race in pserver leading to broken pipe error?

[Eric Siegerman]

On Tue, Feb 10, 2004 at 07:23:43PM -0500, Derek Robert Price wrote:
> Larry Jones wrote:
> >fflush()/close() is a no-no -- you want fclose() instead.
>
> I couldn't find a man page for a file
> descriptor flush -

There isn't one, especially not for pipes.  AFAIK, as soon as
you've written data into a pipe with write(), it's immediately
available for read() without further (necessary or possible)
action on your part.  (For regular files there's fsync(), as
already mentioned, but that's sort of different, in that the
flushing it does is transparent to user processes under all
normal circumstances; for sockets there's shutdown(), which looks
sort of half-way between a flush and a close.)

> is there some way I could have flushed the pipe
> without attaching the stream?

You mean protocol_pipe?  It shouldn't be necessary.  The buffer
that fflush() flushes is only created by fdopen(), so the
combination is a no-op.

stdout and stderr, on the other hand, were already streamified,
and presumably had data written to those streams, so those *do*
need to be flushed -- but see below.

All I think Larry's saying is, "use 'fflush ... fclose' instead
of 'fflush ... close'".  The former sequence gives the userland
stdio layer a chance to clean up everything it needs to clean up,
whereas the latter sequence assumes that stdio has no cleanup to 
do in the first place.  That might well be a correct assumption
on many systems, but it's probably unsafe to count on it.

In fact, I believe you could get away with just fclose()'s, with
no fflush()'s at all.  close() without fflush(), on the other
hand, you know about...  That is:
    fclose(stderr);
    fclose(stdout);
    close(protocol_pipe[1]);

Two other comments:
  - Typo in the comment for set_block_fd() -- s/non-//
        * Set buffer FD to non-blocking I/O.  Returns 0 for success or errno
        * code.

  - Maybe I'm being overly pedantic, but the last bit of new
    code, which drains flowcontrol_pipe, only does so if it
    succeeds in setting the descriptor to blocking mode.  If that
    attempt fails, you could drain the pipe anyway; it'd be a
    busywait loop, but that seems safer than doing nothing.
    Something like this (untested): 

        char junk;
        ssize_t status;
        while ((status=read (flowcontrol_pipe[0], &junk, 1)) > 0
            || (status == -1 && errno == EAGAIN));

--

|  | /\
|-_|/  >   Eric Siegerman, Toronto, Ont.        address@hidden
|  |  /
It must be said that they would have sounded better if the singer
wouldn't throw his fellow band members to the ground and toss the
drum kit around during songs.
        - Patrick Lenneau