git-daemon: Simplify dead-children reaping logic

Move almost all code out of the child_handler() into check_dead_children().
The fact that systemcalls get interrupted by signals allows us to
make the SIGCHLD signal handler almost a no-op by simply running
check_dead_children() right before waiting on poll().

In case some systems do not interrupt systemcalls upon signal receipt,
all zombies will eventually be collected before the next poll() cycle.

Signed-off-by: Stephen R. van den Berg <srb@cuci.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/daemon.c b/daemon.c
index 3e663a877cb8f99b54543a5234a6d992b8abcd63..3abacfd9d923c9b1fd4f2022b247c399c2ad1ee9 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -16,7 +16,6 @@
 static int log_syslog;
 static int verbose;
 static int reuseaddr;
 static int log_syslog;
 static int verbose;
 static int reuseaddr;

-static int child_handler_pipe[2];

 
 static const char daemon_usage[] =
 "git daemon [--verbose] [--syslog] [--export-all]\n"
 
 static const char daemon_usage[] =
 "git daemon [--verbose] [--syslog] [--export-all]\n"


@@ -680,6 +679,21 @@ static void check_dead_children(void)
 {
        unsigned spawned, reaped, deleted;
 
 {
        unsigned spawned, reaped, deleted;
 

+       for (;;) {
+               int status;
+               pid_t pid = waitpid(-1, &status, WNOHANG);
+
+               if (pid > 0) {
+                       unsigned reaped = children_reaped;
+                       if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+                               pid = -pid;
+                       dead_child[reaped % MAX_CHILDREN] = pid;
+                       children_reaped = reaped + 1;
+                       continue;
+               }
+               break;
+       }
+

        spawned = children_spawned;
        reaped = children_reaped;
        deleted = children_deleted;
        spawned = children_spawned;
        reaped = children_reaped;
        deleted = children_deleted;


@@ -760,21 +774,10 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen)
 
 static void child_handler(int signo)
 {
 
 static void child_handler(int signo)
 {

-       for (;;) {
-               int status;
-               pid_t pid = waitpid(-1, &status, WNOHANG);
-
-               if (pid > 0) {
-                       unsigned reaped = children_reaped;
-                       if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
-                               pid = -pid;
-                       dead_child[reaped % MAX_CHILDREN] = pid;
-                       children_reaped = reaped + 1;
-                       write(child_handler_pipe[1], &status, 1);
-                       continue;
-               }
-               break;
-       }
+       /* Otherwise empty handler because systemcalls will get interrupted
+        * upon signal receipt
+        * SysV needs the handler to be rearmed
+        */

        signal(SIGCHLD, child_handler);
 }
 
        signal(SIGCHLD, child_handler);
 }
 


@@ -917,24 +920,21 @@ static int service_loop(int socknum, int *socklist)
        struct pollfd *pfd;
        int i;
 
        struct pollfd *pfd;
        int i;
 

-       if (pipe(child_handler_pipe) < 0)
-               die ("Could not set up pipe for child handler");
-
-       pfd = xcalloc(socknum + 1, sizeof(struct pollfd));
+       pfd = xcalloc(socknum, sizeof(struct pollfd));

 
        for (i = 0; i < socknum; i++) {
                pfd[i].fd = socklist[i];
                pfd[i].events = POLLIN;
        }
 
        for (i = 0; i < socknum; i++) {
                pfd[i].fd = socklist[i];
                pfd[i].events = POLLIN;
        }

-       pfd[socknum].fd = child_handler_pipe[0];
-       pfd[socknum].events = POLLIN;

 
        signal(SIGCHLD, child_handler);
 
        for (;;) {
                int i;
 
 
        signal(SIGCHLD, child_handler);
 
        for (;;) {
                int i;
 

-               if (poll(pfd, socknum + 1, -1) < 0) {
+               check_dead_children();
+
+               if (poll(pfd, socknum, -1) < 0) {

                        if (errno != EINTR) {
                                logerror("Poll failed, resuming: %s",
                                      strerror(errno));
                        if (errno != EINTR) {
                                logerror("Poll failed, resuming: %s",
                                      strerror(errno));


@@ -942,10 +942,6 @@ static int service_loop(int socknum, int *socklist)
                        }
                        continue;
                }
                        }
                        continue;
                }

-               if (pfd[socknum].revents & POLLIN) {
-                       read(child_handler_pipe[0], &i, 1);
-                       check_dead_children();
-               }

 
                for (i = 0; i < socknum; i++) {
                        if (pfd[i].revents & POLLIN) {
 
                for (i = 0; i < socknum; i++) {
                        if (pfd[i].revents & POLLIN) {


@@ -1036,11 +1032,6 @@ int main(int argc, char **argv)
        gid_t gid = 0;
        int i;
 
        gid_t gid = 0;
        int i;
 

-       /* Without this we cannot rely on waitpid() to tell
-        * what happened to our children.
-        */
-       signal(SIGCHLD, SIG_DFL);
-

        for (i = 1; i < argc; i++) {
                char *arg = argv[i];
 
        for (i = 1; i < argc; i++) {
                char *arg = argv[i];