From: Fredrik Tolf <fredrik@dolda2000.com>
Date: Mon, 13 Oct 2008 04:01:18 +0000 (+0200)
Subject: Fixed possible child wait race condition.
X-Git-Tag: 1.3~11^2^2~1
X-Git-Url: http://git.dolda2000.com/gitweb/?a=commitdiff_plain;h=bc222c94b892d2be70ef393b1523a3f48eaa60ee;p=doldaconnect.git

Fixed possible child wait race condition.
---

diff --git a/daemon/main.c b/daemon/main.c
index 58d1c6c..463d8be 100644
--- a/daemon/main.c
+++ b/daemon/main.c
@@ -91,7 +91,6 @@ void childcallback(pid_t pid, void (*func)(pid_t, int, void *), void *data)
     new->pid = pid;
     new->callback = func;
     new->data = data;
-    new->finished = 0;
     new->prev = NULL;
     new->next = children;
     if(children != NULL)
@@ -139,9 +138,6 @@ static void terminate(void)
 
 static void handler(int signum)
 {
-    pid_t pid;
-    int status;
-    struct child *child;
     FILE *dumpfile;
     extern int numfnetnodes, numtransfers, numdcpeers;
     
@@ -155,18 +151,7 @@ static void handler(int signum)
 	running = 0;
 	break;
     case SIGCHLD:
-	while((pid = waitpid(-1, &status, WNOHANG)) > 0)
-	{
-	    for(child = children; child != NULL; child = child->next)
-	    {
-		if(child->pid == pid)
-		{
-		    child->finished = 1;
-		    child->status = status;
-		}
-	    }
-	    childrendone = 1;
-	}
+	childrendone = 1;
 	break;
     case SIGUSR1:
 	flog(LOG_NOTICE, "forking and dumping core upon SIGUSR1");
@@ -185,6 +170,32 @@ static void handler(int signum)
     }
 }
 
+static void checkchildren(void)
+{
+    pid_t pid;
+    int status;
+    struct child *child;
+
+    while((pid = waitpid(-1, &status, WNOHANG)) > 0)
+    {
+	for(child = children; child != NULL; child = child->next)
+	{
+	    if(child->pid == pid)
+	    {
+		child->callback(pid, status, child->data);
+		if(child == children)
+		    children = child->next;
+		if(child->prev != NULL)
+		    child->prev->next = child->next;
+		if(child->next != NULL)
+		    child->next->prev = child->prev;
+		free(child);
+		break;
+	    }
+	}
+    }
+}
+
 pid_t forksess(uid_t user, struct authhandle *auth, void (*ccbfunc)(pid_t, int, void *), void *data, ...)
 {
     int i, o;
@@ -383,7 +394,6 @@ int main(int argc, char **argv)
     int delay, immsyslog;
     struct module *mod;
     struct timer *timer;
-    struct child *child;
     double now;
     
     now = ntime();
@@ -534,11 +544,10 @@ int main(int argc, char **argv)
 		    delay = (int)((timer->at - now) * 1000.0);
 	    }
 	}
+	/* Of course, there's a race condition here that should be
+	 * solved with pselect, but it doesn't matter a lot. */
 	if(childrendone)
-	{
 	    delay = 0;
-	    childrendone = 0;
-	}
 	pollsocks(delay);
 	now = ntime();
 	do
@@ -558,24 +567,11 @@ int main(int argc, char **argv)
 		break;
 	    }
 	} while(timer != NULL);
-	do
+	if(childrendone)
 	{
-	    for(child = children; child != NULL; child = child->next)
-	    {
-		if(child->finished)
-		{
-		    child->callback(child->pid, child->status, child->data);
-		    if(child == children)
-			children = child->next;
-		    if(child->prev != NULL)
-			child->prev->next = child->next;
-		    if(child->next != NULL)
-			child->next->prev = child->prev;
-		    free(child);
-		    break;
-		}
-	    }
-	} while(child != NULL);
+	    childrendone = 0;
+	    checkchildren();
+	}
     }
     flog(LOG_INFO, "terminating...");
     terminate();
diff --git a/daemon/sysevents.h b/daemon/sysevents.h
index 141fa6e..86cf69f 100644
--- a/daemon/sysevents.h
+++ b/daemon/sysevents.h
@@ -40,8 +40,6 @@ struct child
     pid_t pid;
     void (*callback)(pid_t pid, int status, void *data);
     void *data;
-    int status;
-    volatile int finished;
 };
 
 void childcallback(pid_t pid, void (*func)(pid_t, int, void *), void *data);