-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Steve McIntyre wrote:
>On Mon, Feb 09, 2004 at 05:31:07PM -0500, Larry Jones wrote: > >>Steve McIntyre writes: >> >>>http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=24253 >>> >>>They had a similar problem with the Debian package, and the patch >>>listed on that page seems to fix it for them. >>> >>>Thoughts? >> >>My first impression is that it seems reasonable, but I'll have to give >>it some thought. Note that the patch is defective, however: In the >>definition of unset_nonblock_fd, the ``|'' should be ``&''. > > >Hmm, yes. Of course it should. I'm a little curious as to how/why this >patch actually helps people now... It may simply be that they put the patch in without being able to test it because they couldn't reproduce the problem. Anyhow, I think the diagnosis is correct. I added a fix to the buffer close routines back in October 2002 due to a problem with server child processes sending a SIGPIPE's instead of a SIGCHILD intermittantly. I suspected a race condition but we could never nail down the cause. The old log messages and discussions of the problem I just reviewed would corroborate this theory exactly. I've attempted to rewrite this patch, and attached it, still broken, only because I may have to leave the office in a few minutes. remotecheck is curerntly failing with this patch because STDOUT isn't flushing, it looks like. Derek Index: src/ChangeLog =================================================================== RCS file: /cvs/ccvs/src/ChangeLog,v retrieving revision 1.2336.2.176 diff -u -r1.2336.2.176 ChangeLog - --- src/ChangeLog 10 Feb 2004 19:54:59 -0000 1.2336.2.176 +++ src/ChangeLog 10 Feb 2004 21:37:42 -0000 @@ -1,5 +1,15 @@ 2004-02-10 Derek Price <[EMAIL PROTECTED]> + * server.c (do_cvs_command): Have the server child close all the pipes + but the flow control pipe and wait on an EOF on the flow control pipe + from the parent when done to avoid a race condition that could + otherwise generate a SIGPIPE for the parent before the SIGCHILD when + the other pipes were so full after a child exited that the parent + attempted to write a stop byte to the flow control pipe. + (Original report from <[EMAIL PROTECTED]>.) + +2004-02-10 Derek Price <[EMAIL PROTECTED]> + * buffer.c (stdio_buffer_shutdown): Add a helpful comment. 2004-02-09 Derek Price <[EMAIL PROTECTED]> Index: src/server.c =================================================================== RCS file: /cvs/ccvs/src/server.c,v retrieving revision 1.284.2.15 diff -u -r1.284.2.15 server.c - --- src/server.c 2 Feb 2004 15:12:11 -0000 1.284.2.15 +++ src/server.c 10 Feb 2004 21:37:46 -0000 @@ -2293,13 +2293,18 @@ # define SERVER_LO_WATER (1 * 1024 * 1024) # endif /* SERVER_LO_WATER */ + + +/* Protos */ static int set_nonblock_fd PROTO((int)); +static int set_block_fd PROTO((int)); + + /* - - * Set buffer BUF to non-blocking I/O. Returns 0 for success or errno + * Set buffer FD to non-blocking I/O. Returns 0 for success or errno * code. */ - - static int set_nonblock_fd (fd) int fd; @@ -2314,8 +2319,29 @@ return 0; } + + +/* + * Set buffer FD to non-blocking I/O. Returns 0 for success or errno + * code. + */ +static int +set_block_fd (fd) + int fd; +{ + int flags; + + flags = fcntl (fd, F_GETFL, 0); + if (flags < 0) + return errno; + if (fcntl (fd, F_SETFL, flags & ~O_NONBLOCK) < 0) + return errno; + return 0; +} #endif /* SERVER_FLOWCONTROL */ - - + + + static void serve_questionable PROTO((char *)); static void @@ -2792,11 +2818,33 @@ /* For now we just discard partial lines on stderr. I suspect that CVS can't write such lines unless there is a bug. */ - - /* - - * When we exit, that will close the pipes, giving an EOF to - - * the parent. - - */ buf_free (protocol); + + /* Close the pipes explicitly in order to send an EOF to the parent, + * then wait for the parent to close the flow control pipe. This + * avoids a race condition where a child which dumped more than the + * high water mark into the pipes could complete its job and exit, + * leaving the parent process to attempt to write a stop byte to the + * closed flow control pipe, which earned the parent a SIGPIPE, which + * it normally only expects on the network pipe and that causes it to + * exit with an error message, rather than the SIGCHILD that it knows + * how to handle correctly. + */ + /* Let exit() close STDIN - it's from /dev/null anyhow. */ + close (STDERR_FILENO); + close (STDOUT_FILENO); + close (protocol_pipe[1]); +#ifdef SERVER_FLOWCONTROL + if (set_block_fd (flowcontrol_pipe[0]) == 0) + { + char junk; + while (read (flowcontrol_pipe[0], &junk, 1) != 0); + } + /* FIXCVS: No point in printing an error message with error(), + * as STDERR is already closed, but perhaps this could be syslogged? + */ +#endif + exit (exitstatus); } - -- *8^) Email: [EMAIL PROTECTED] 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 iD8DBQFAKU+VLD1OTBfyMaQRAtEiAKDbNV+2iKX9S3RTauc8tWPamdO6OACfWypb saBnvEDyoGbu/7Omvx+kjys= =c1ER -----END PGP SIGNATURE----- _______________________________________________ Info-cvs mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/info-cvs
