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

[Derek Robert Price]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Eric Siegerman wrote:

>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.


Okay... I had just made the assumption that all the descriptors would
need to be flushed before close if one had, but I see what you are
getting at now.  The stdout stream had been used and has its own buffer
so I needed to flush the stdout buf, and similarly the stderr buf, but
not the descriptors.

>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.


Actually, it apparently isn't safe - the problem I ran into was that
closing the stdout descriptor and not the stream lost some data.

>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));


I've made these changes and removed the set_block_fd() function
completely as it is no longer needed.

Thanks.  Testing these changes now but things look good and I should
commit it soon.

Derek

- --
                *8^)

Email: address@hidden

Get CVS support at <http://ximbiot.com>!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Netscape - http://enigmail.mozdev.org

iD8DBQFAKk3VLD1OTBfyMaQRAtpkAKCDhMjbX2Qqgeyn8ACPevNT2DJt7gCfUdCG
4WOOKMAFx5a4ABNsgJ/vLUA=
=AKf/
-----END PGP SIGNATURE-----