git daemon: avoid calling syslog() from a signal handler

Signal handlers should never call syslog(), as that can raise signals
of its own.

Instead, call the syslog() from the master process.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 50 insertions, 20 deletions

diff --git a/daemon.c b/daemon.c
index 63cd12c..ce3a6f5 100644
--- a/daemon.c
+++ b/daemon.c
@@ -694,23 +694,47 @@ static void kill_some_children(int signo, unsigned start, unsigned stop)
 	}
 }
 
+static void check_dead_children(void)
+{
+	unsigned spawned, reaped, deleted;
+
+	spawned = children_spawned;
+	reaped = children_reaped;
+	deleted = children_deleted;
+
+	while (deleted < reaped) {
+		pid_t pid = dead_child[deleted % MAX_CHILDREN];
+		const char *dead = pid < 0 ? " (with error)" : "";
+
+		if (pid < 0)
+			pid = -pid;
+
+		/* XXX: Custom logging, since we don't wanna getpid() */
+		if (verbose) {
+			if (log_syslog)
+				syslog(LOG_INFO, "[%d] Disconnected%s",
+						pid, dead);
+			else
+				fprintf(stderr, "[%d] Disconnected%s\n",
+						pid, dead);
+		}
+		remove_child(pid, deleted, spawned);
+		deleted++;
+	}
+	children_deleted = deleted;
+}
+
 static void check_max_connections(void)
 {
 	for (;;) {
 		int active;
-		unsigned spawned, reaped, deleted;
+		unsigned spawned, deleted;
+
+		check_dead_children();
 
 		spawned = children_spawned;
-		reaped = children_reaped;
 		deleted = children_deleted;
 
-		while (deleted < reaped) {
-			pid_t pid = dead_child[deleted % MAX_CHILDREN];
-			remove_child(pid, deleted, spawned);
-			deleted++;
-		}
-		children_deleted = deleted;
-
 		active = spawned - deleted;
 		if (active <= max_connections)
 			break;
@@ -760,18 +784,10 @@ static void child_handler(int signo)
 
 		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;
-			/* XXX: Custom logging, since we don't wanna getpid() */
-			if (verbose) {
-				const char *dead = "";
-				if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
-					dead = " (with error)";
-				if (log_syslog)
-					syslog(LOG_INFO, "[%d] Disconnected%s", pid, dead);
-				else
-					fprintf(stderr, "[%d] Disconnected%s\n", pid, dead);
-			}
 			continue;
 		}
 		break;
@@ -928,8 +944,18 @@ static int service_loop(int socknum, int *socklist)
 
 	for (;;) {
 		int i;
+		int timeout;
 
-		if (poll(pfd, socknum, -1) < 0) {
+		/*
+		 * This 1-sec timeout could lead to idly looping but it is
+		 * here so that children culled in child_handler() are reported
+		 * without too much delay.  We could probably set up a pipe
+		 * to ourselves that we poll, and write to the fd from child_handler()
+		 * to wake us up (and consume it when the poll() returns...
+		 */
+		timeout = (children_spawned != children_deleted) ? 1000 : -1;
+		i = poll(pfd, socknum, timeout);
+		if (i < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));
@@ -937,6 +963,10 @@ static int service_loop(int socknum, int *socklist)
 			}
 			continue;
 		}
+		if (i == 0) {
+			check_dead_children();
+			continue;
+		}
 
 		for (i = 0; i < socknum; i++) {
 			if (pfd[i].revents & POLLIN) {