Prevent infinite recursion in rwrite()

Dave Dykstra dwd at bell-labs.com
Wed May 8 12:58:05 EST 2002


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




More information about the rsync mailing list