Git development
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Brian Foster <brian.foster@innova-card.com>
Cc: git@vger.kernel.org
Subject: [PATCH] git daemon: avoid calling syslog() from a signal handler
Date: Thu, 3 Jul 2008 16:27:24 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.1.00.0807031624020.9925@racer> (raw)
In-Reply-To: <alpine.DEB.1.00.0807031531320.9925@racer>


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>
---

	On Thu, 3 Jul 2008, Johannes Schindelin wrote:

	> It may raise awareness so much that somebody gets a clever idea 
	> how to cope with it.

	Okay, it might not be clever, but I think this is pretty 
	straight-forward.

	However, this part of the code is tricky, as it can (and will) be 
	interrupted by signal handlers, so I would appreciate several 
	careful reviews (but maybe it is not necessary to ask for it, 
	since I am no longer trusted).

 daemon.c |   61 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/daemon.c b/daemon.c
index 63cd12c..35fd439 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;
@@ -929,7 +945,8 @@ static int service_loop(int socknum, int *socklist)
 	for (;;) {
 		int i;
 
-		if (poll(pfd, socknum, -1) < 0) {
+		i = poll(pfd, socknum, 1);
+		if (i < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));
@@ -937,6 +954,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) {
-- 
1.5.6.1.376.g6b0fd

  reply	other threads:[~2008-07-03 15:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-03 12:00 Non-inetd git-daemon hangs in syslog(3)/fclose(3) if --syslog --verbose accessing non-repositories Brian Foster
2008-07-03 12:45 ` Johannes Schindelin
2008-07-03 13:52   ` Brian Foster
2008-07-03 14:32     ` Johannes Schindelin
2008-07-03 15:27       ` Johannes Schindelin [this message]
2008-07-05  7:34         ` [PATCH] git daemon: avoid calling syslog() from a signal handler Junio C Hamano
2008-07-05 10:05           ` Johannes Schindelin
2008-07-05 17:26             ` Junio C Hamano
2008-07-06  1:42               ` Johannes Schindelin
2008-07-06  6:08                 ` Junio C Hamano
2008-07-06 12:23                   ` [PATCH v2] " Johannes Schindelin
2008-07-07  1:50                     ` Junio C Hamano
2008-07-07  6:54                       ` Stephen R. van den Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.00.0807031624020.9925@racer \
    --to=johannes.schindelin@gmx.de \
    --cc=brian.foster@innova-card.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox