On Mon, May 06, 2002 at 05:49:28PM -0700, Wayne Davison
wrote:> Here's a resend of an old patch that is intended to avoid an infinite
> recursion (ending in a stack overflow) of the rwrite() function getting
> an error that calls rwrite(), ad naseum. I've only seen this happen
> when one of the sides dies due to a program error -- in that case, the
> connection is closed, and when we try to send an error to the other
> side and it generates an error, the error generates an error, etc.
>
> My solution is to use a simple static variable as a semaphore. If we
> get back to rwrite() with a non-zero value, we never again try to send
> a message over the socket. This results in the error going out to
> stderr. In the problem case I saw, this resulted in an error message
> being displayed on my terminal (2 actually) instead of a weird crash.
>
> ..wayne..
>
> ---8<------8<------8<------8<---cut
here--->8------>8------>8------>8---
> Index: log.c
> --- log.c 2002/04/08 09:10:50 1.61
> +++ log.c 2002/05/07 00:32:30
> @@ -215,6 +215,7 @@
> void rwrite(enum logcode code, char *buf, int len)
> {
> FILE *f=NULL;
> + static char semaphore = 0;
> extern int am_daemon;
> extern int am_server;
> extern int quiet;
> @@ -243,8 +244,11 @@
> * io_multiplex_write can fail if we do not have a multiplexed
> * connection at the moment, in which case we fall through and
> * log locally instead. */
> - if (am_server && io_multiplex_write(code, buf, len)) {
> - return;
> + if (am_server && (!semaphore++)) {
> + int ret = io_multiplex_write(code, buf, len);
> + semaphore--;
> + if (ret)
> + return;
> }
>
> if (am_daemon) {
I wouldn't use the name "semaphore" because it isn't very
descriptive.
Maybe something like "nestlevel" or "recurselevel". Also,
the reason for
that code is non-intuitive so I think a comment about what problem it's
trying to solve would be a good idea.
I'm a little nervous about whether or not there might be some cases where
recursion of this function is normal. I think it might be a good idea to
defer this to 2.6.0 instead of 2.5.6; I think 2.5.6 should only have
low-risk things because we want a stable release for a change. I think
Martin has already made a CVS branch.
- Dave Dykstra