[PATCH] rsync kills all user processes on fork failure

[Paul Haas]

I filed this as bug 4150 http://rsync.samba.org/cgi-bin/rsync/

We had a few hundred hung processes and reached the limit for "max user
processes" (see the ulimit command).   rsync did not handle this very
well.  It passed -1 as a PID to kill().  When kill() gets -1, it kills all
of the processes owned by that user, or if run by root, all processes.

I attached a one line fix.  Let me know if I used the wrong format for a
patch.  If the result of fork is -1 don't put it in the list of pids to
kill.

Reviewing the mailing list, I saw that this happened to at least one other
person, see http://lists.samba.org/pipermail/rsync/2001-April/004158.html
Back in April of 2001, Randy Kramer wrote:

> The rsync server often quit for me when I used the -c option,
> generally because something on the server end decided to kill rsync.

I duplicated the problem with version 2.5.4 on a Linux system.  I used
another user ID, "notme", so that when rsync killed all the processes, it
wouldn't kill my shell.  "ulimit -u 5" sets the max number of user
processes to 5.  I used the sleep command as my victim processes, it's
annoying to have something usefull killed when duplicating a bug.

bash$ su notme
Password:
bash$ ulimit -u 5
bash$ sleep 100 & sleep 100 & sleep 100 &
[1] 16234
[2] 16235
[3] 16236
bash$ # There are now 4 processes running as "notme", bash and 3 sleeps.
bash$ whoami
notme
bash$ ./rsync -e /usr/bin/rsh -a ~/a bandit:b
fork: Resource temporarily unavailable
rsync error: error in IPC code (code 14) at util.c(137)
bash$ whoami
me
bash$ # all of the notme processes were killed.


None of this has anything to do with the bug in our scripts that lead to
hundreds of hung processes.


===================================================================
RCS file: RCS/util.c,v
retrieving revision 1.1
diff -u -r1.1 util.c
--- util.c      2002/03/19 15:01:28     1.1
+++ util.c      2002/03/19 15:29:32
@@ -483,7 +483,7 @@
 {
        pid_t newpid = fork();

-       if (newpid) {
+       if (newpid > 0) {
                all_pids[num_pids++] = newpid;
        }
        return newpid;

--
Paul Haas, paulh at hamjudo.com http://hamjudo.com