I couldn't reproduce the problem on Linux, but I believe you that it's
a problem. If you think about it, it's easy to see why code that called
_exit_cleanup() might behave strangely if the function returns so I
like the fix of not returning until the situation gets deeply nested.
I submitted the patch.
Thanks Marc & Brian.
- Dave Dykstra
On Thu, Jan 23, 2003 at 08:29:40PM -0500, Brian Poole
wrote:> Hello,
>
> There is still a problem with the _exit_cleanup() function in
> cleanup.c despite the patch that was put in last week that prevented
> recursion. It turns out that sometimes multiple calls in close
> sequence are done and this causes rsync to spin out of control
> instead of exiting.
>
> This bug was found by Marc Espie and the patch I'm including is
> his as well. It hasn't been commited to the OpenBSD tree yet and
> is still undergoing testing but it does fix the immediate easily
> detectable problem. His post with the bug fix was here:
>
> http://marc.theaimsgroup.com/?l=openbsd-tech&m=104238879828005&w=2
>
> I've changed the variable names to match what is in the rsync
> tree already.
>
> This bug is easily triggerable with a local rsync copy.
>
> rsync -av /some/reasonably/large/dir /tmp/somewhere
>
> Wait until after the copy has started (after it is done with
> building the file list) and then hit ctrl-c. Watch rsync fill
> the screen with messages like:
>
> rsync: connection unexpectedly closed (94819 bytes read so far)
> rsync: connection unexpectedly closed (94819 bytes read so far)
> rsync: connection unexpectedly closed (94819 bytes read so far)
>
> Curse.
> Switch to another window and kill -9 the errant rsync process.
> Apply patch and notice it no longer goes crazy.
>
>
> -b
>
> Index: cleanup.c
> ==================================================================> RCS
file: /cvsroot/rsync/cleanup.c,v
> retrieving revision 1.15
> diff -u -r1.15 cleanup.c
> --- cleanup.c 16 Jan 2003 20:09:31 -0000 1.15
> +++ cleanup.c 24 Jan 2003 01:24:09 -0000
> @@ -63,11 +63,11 @@
> extern int log_got_error;
> static int inside_cleanup = 0;
>
> - if (inside_cleanup != 0) {
> + if (inside_cleanup > 10) {
> /* prevent the occasional infinite recursion */
> return;
> }
> - inside_cleanup = 1;
> + inside_cleanup++;
>
> signal(SIGUSR1, SIG_IGN);
> signal(SIGUSR2, SIG_IGN);
> Index: cleanup.c
> ==================================================================> RCS
file: /cvsroot/rsync/cleanup.c,v
> retrieving revision 1.15
> diff -u -r1.15 cleanup.c
> --- cleanup.c 16 Jan 2003 20:09:31 -0000 1.15
> +++ cleanup.c 24 Jan 2003 01:24:09 -0000
> @@ -63,11 +63,11 @@
> extern int log_got_error;
> static int inside_cleanup = 0;
>
> - if (inside_cleanup != 0) {
> + if (inside_cleanup > 10) {
> /* prevent the occasional infinite recursion */
> return;
> }
> - inside_cleanup = 1;
> + inside_cleanup++;
>
> signal(SIGUSR1, SIG_IGN);
> signal(SIGUSR2, SIG_IGN);