Non-inetd git-daemon hangs in syslog(3)/fclose(3) if --syslog --verbose accessing non-repositories

Non-inetd git-daemon hangs in syslog(3)/fclose(3) if --syslog --verbose accessing non-repositories

[Brian Foster]

 I've seen several reports of what seems to be the following
 problem, but no fixes.  I do not understand the root-cause.
 I have found, however, what seems to be a work-around.

 I'm starting v1.5.2.5 (the Kubuntu 7.10 package) git-daemon
 (as a normal-user, *not* super-user) as:

     export GIT_TRACE=/tmp/LOG-git-daemon
     exec  \
        git daemon --detach --syslog --verbose --base-path=/pub/scm

 The repositories being served are simple (non-bare) clones,
 with nothing strange/weird.  The remote machine (CentOS),
 running a self-built un-modified v1.5.5, can access them Ok:

     $ git ls-remote git://SERVER/repo
     ... works ...
     $ 

 However, an invalid path causes both the git-daemon and the
 client to hang (I'm too impatient and do not know if either
 times out):

     $ git ls-remote git://SERVER/repo/garbage
     ... hangs ...

 I can ^C the client, but the server is still hung, and will
 not respond to *any* requests.  I must kill the git-daemon.
 The GIT_TRACE log's contents do not seem to be interesting.
 The last entries in the syslog are:

     git-daemon: [3705] Connection from <REMOTE>
     git-daemon: [3705] Extended attributes (17 bytes) exist <...>
     git-daemon: [3705] Request upload-pack for '/repo/garbage'
     git-daemon: [3705] '/pub/scm/repo/garbage': unable to chdir or not a git archive

 Annoyingly, strace(1)ing git-daemon causes the problem to
 vanish!  Everything then seems to be work as expected.
 But now the syslog contains an additional, 5th, line:

     git-daemon: [3705] Disconnected (with error)

 (The "with error" is present only in the .../garbage case.)

 Attaching to a hung git-daemon with strace shows that it's
 hung in futex(2):

     $ strace -p3705
     Process 3705 attached - interrupt to quit
     futex(0x2b502cbf3980, FUTEX_WAIT, 2, NULL <unfinished ...>
     ... hangs ...

 Attaching to a hung git-daemon with gdb(1) results in the
 following backtrace:

     (gdb) where
     #0  0x00002b502c97d1d8 in ?? () from /lib/libc.so.6
     #1  0x00002b502c913698 in ?? () from /lib/libc.so.6
     #2  0x00002b502c912960 in realloc () from /lib/libc.so.6
     #3  0x00002b502c905eb4 in ?? () from /lib/libc.so.6
     #4  0x00002b502c8fd897 in fclose () from /lib/libc.so.6
     #5  0x00002b502c96d371 in __vsyslog_chk () from /lib/libc.so.6
     #6  0x00002b502c96d8a0 in syslog () from /lib/libc.so.6
     #7  0x0000000000403896 in ?? ()
     #8  <signal handler called>
     #9  0x00002b502c9373ab in fork () from /lib/libc.so.6
     #10 0x0000000000404443 in ?? ()
     #11 0x0000000000404c99 in ?? ()
     #12 0x00002b502c8bab44 in __libc_start_main () from /lib/libc.so.6
     #13 0x0000000000403179 in ?? ()
     #14 0x00007fff7e63eab8 in ?? ()
     #15 0x0000000000000000 in ?? ()
     (gdb) 

 It's fairly clear it's hung doing the syslog(3) of the (missing)
 "Disconnected (with error)" message.  A interesting point is the
 SIGCHLD handler in (git-)daemon.c appears to have been called
 before the parent's fork(2) returned.  I presume there is a race
 here, but admit I do not see it.  This (broadly) makes sense; the
 server is a Very Fast machine.  And strace'ing slows things down.

 The workaround is to omit --verbose and hence never try to syslog
 the "Disconnected ..." message.

cheers!
	-blf-

-- 
“How many surrealists does it take to   | Brian Foster
 change a lightbulb? Three. One calms   | somewhere in south of France
 the warthog, and two fill the bathtub  |   Stop E$$o (ExxonMobil)!
 with brightly-coloured machine tools.” |      http://www.stopesso.com

Re: Non-inetd git-daemon hangs in syslog(3)/fclose(3) if --syslog --verbose accessing non-repositories

[Johannes Schindelin]

Hi,

On Thu, 3 Jul 2008, Brian Foster wrote:

>  I've seen several reports of what seems to be the following
>  problem, but no fixes.
>
> [describes that git-daemon -v syslog()s in a signal handler, which is 
>  unsupported]

I reported this bug earlier, and my workaround was to comment out the 
syslog() in the signal handler, but I have no real fix for that, either.

Unfortunately, the wise people on this list did not have an idea either, 
at least they did not share it with me.

Ciao,
Dscho

Re: Non-inetd git-daemon hangs in syslog(3)/fclose(3) if --syslog --verbose accessing non-repositories

[Brian Foster]

On Thursday 03 July 2008 Johannes Schindelin wrote:
> On Thu, 3 Jul 2008, Brian Foster wrote:
> >[... describes that git-daemon -v syslog()s in a signal handler,
> >  which is unsupported ...]
> 
> I reported this bug earlier [ ... ]

 Ah, yes, I've (now) found the (long!) thread,
 about log-rotation (which, as you observe, is
 not the problem).  Sorry for the duplication.

cheers!
	-blf-

-- 
“How many surrealists does it take to   | Brian Foster
 change a lightbulb? Three. One calms   | somewhere in south of France
 the warthog, and two fill the bathtub  |   Stop E$$o (ExxonMobil)!
 with brightly-coloured machine tools.” |      http://www.stopesso.com

Re: Non-inetd git-daemon hangs in syslog(3)/fclose(3) if --syslog --verbose accessing non-repositories

[Johannes Schindelin]

Hi,

On Thu, 3 Jul 2008, Brian Foster wrote:

> On Thursday 03 July 2008 Johannes Schindelin wrote:
> > On Thu, 3 Jul 2008, Brian Foster wrote:
> > >[... describes that git-daemon -v syslog()s in a signal handler,
> > >  which is unsupported ...]
> > 
> > I reported this bug earlier [ ... ]
> 
>  Ah, yes, I've (now) found the (long!) thread, about log-rotation 
>  (which, as you observe, is not the problem).

Yeah, sorry, should have mentioned that.

> Sorry for the duplication.

No need to be sorry.  It may raise awareness so much that somebody gets a 
clever idea how to cope with it.

Ciao,
Dscho

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

[Johannes Schindelin]

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

Re: [PATCH] git daemon: avoid calling syslog() from a signal handler

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Signal handlers should never call syslog(), as that can raise signals
> of its own.
>
> Instead, call the syslog() from the master process.

Earlier parts seem to make sense but I am puzzled by these changes.

> @@ -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;
> +		}

So you will check every 1ms to see if there are new dead children, but why
is this necessary?

Re: [PATCH] git daemon: avoid calling syslog() from a signal handler

[Johannes Schindelin]

Hi,

On Sat, 5 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Signal handlers should never call syslog(), as that can raise signals 
> > of its own.
> >
> > Instead, call the syslog() from the master process.
> 
> Earlier parts seem to make sense but I am puzzled by these changes.
> 
> > @@ -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;
> > +		}
> 
> So you will check every 1ms to see if there are new dead children, but why
> is this necessary?

This comes from me not reading the man page for poll() properly.  Of 
course, I want to check every second: syslog timestamps the messages with 
a resolution of 1 second, AFAIR, or at least some of them do.

So if you could just squash in this patch, that would be smashing:

-- snipsnap --
@@ -945,8 +945,8 @@ static int service_loop(int socknum, int *socklist)
 	for (;;) {
 		int i;
 
-		i = poll(pfd, socknum, 1);
+		i = poll(pfd, socknum, 1000);
 		if (i < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));

Re: [PATCH] git daemon: avoid calling syslog() from a signal handler

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> So you will check every 1ms to see if there are new dead children, but why
>> is this necessary?
>
> This comes from me not reading the man page for poll() properly.  Of 
> course, I want to check every second: syslog timestamps the messages with 
> a resolution of 1 second, AFAIR, or at least some of them do.

Hmm.

The question was not about the millisecond typo, but about why time-out at
all.

We would need to somehow break out of poll() after handling the SIGCHLD
signal and I guess timing the syscall out would be the most obvious way,
but somehow it felt awkward.

Another way would be to set up a pipe to ourself that is included in the
poll() and write a byte to the pipe from the signal handler.

Re: [PATCH] git daemon: avoid calling syslog() from a signal handler

[Johannes Schindelin]

Hi,

On Sat, 5 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> So you will check every 1ms to see if there are new dead children, 
> >> but why is this necessary?
> >
> > This comes from me not reading the man page for poll() properly.  Of 
> > course, I want to check every second: syslog timestamps the messages 
> > with a resolution of 1 second, AFAIR, or at least some of them do.
> 
> Hmm.
> 
> The question was not about the millisecond typo, but about why time-out 
> at all.

Because I do not want to change the semantics!

ATM, in those cases where it works (as opposed to hanging!), git-daemon 
--verbose reports in the syslog when a client disconnected, possibly with 
an error.  It does so with a timestamp so that you can see how long the 
connection lasted.  That is what logs are useful for.

Now, syslog has timestamps at second-resolution (at least here it does), 
and I wanted to imitate that.

The alternative would be to deprive all users of an (mostly) accurate 
timestamp of the disconnect time.

> Another way would be to set up a pipe to ourself that is included in the 
> poll() and write a byte to the pipe from the signal handler.

It still would need to break out of the poll(), in which case the effect 
would be _exactly_ the same, but with a lot of more trouble, and 
opportunities for me to bring in new bugs, right?

Ciao,
Dscho

Re: [PATCH] git daemon: avoid calling syslog() from a signal handler

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> The question was not about the millisecond typo, but about why time-out 
>> at all.
>
> Because I do not want to change the semantics!
>
> ATM, in those cases where it works (as opposed to hanging!), git-daemon 
> --verbose reports in the syslog when a client disconnected, possibly with 
> an error.  It does so with a timestamp so that you can see how long the 
> connection lasted.  That is what logs are useful for.
>
> Now, syslog has timestamps at second-resolution (at least here it does), 
> and I wanted to imitate that.

Yes, we do not want to change the semantics, but that is not a reason for
an unused daemon to wake up every second, isn't it?

> The alternative would be to deprive all users of an (mostly) accurate 
> timestamp of the disconnect time.
>
>> Another way would be to set up a pipe to ourself that is included in the 
>> poll() and write a byte to the pipe from the signal handler.
>
> It still would need to break out of the poll(), in which case the effect 
> would be _exactly_ the same,...

I do not think so.

In the solution I suggested, you would set up a pipe to yourself, and give
the read end of the pipe and the accepting socket to poll() with infinite
timeout.  And when you do reap in the signal handler, you write a byte to
the write end of the pipe (and that would be the only codepath that would
write to that pipe).  That would make the pipe you are polling redable,
and allow you to break out of poll().  When poll() returns thusly, you
notice you are in that condition and read out that byte to keep the pipe
clean, and do the check_dead_children() thing.

That way, you would wake up only when you actually have something useful
to do.  Otherwise you will stay dormant, waiting for something to happen,
either by socket becoming ready to accept, or child dying and raising
SIGCHLD.

I agree that it is a bit too elaborate change that we do not want to have
in 'maint'.

Even then, for 'maint', we probably would at least want something like
this on top of your patch, so that when we know we have absolutely nothing
to do, we do not have to spin.

 daemon.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/daemon.c b/daemon.c
index 6dc96aa..ce3a6f5 100644
--- a/daemon.c
+++ b/daemon.c
@@ -944,6 +944,7 @@ static int service_loop(int socknum, int *socklist)
 
 	for (;;) {
 		int i;
+		int timeout;
 
 		/*
 		 * This 1-sec timeout could lead to idly looping but it is
@@ -952,7 +953,8 @@ static int service_loop(int socknum, int *socklist)
 		 * to ourselves that we poll, and write to the fd from child_handler()
 		 * to wake us up (and consume it when the poll() returns...
 		 */
-		i = poll(pfd, socknum, 1000);
+		timeout = (children_spawned != children_deleted) ? 1000 : -1;
+		i = poll(pfd, socknum, timeout);
 		if (i < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",

[PATCH v2] git daemon: avoid calling syslog() from a signal handler

[Johannes Schindelin]

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

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

To avoid waking up unnecessarily, a pipe is set up that is only ever
written to by child_handler(), when a child disconnects, as suggested
per Junio.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sat, 5 Jul 2008, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > Junio wrote:
	> >> Another way would be to set up a pipe to ourself that is 
	> >> included in the poll() and write a byte to the pipe from the signal 
	> >> handler.
	> >
	> > It still would need to break out of the poll(), in which case 
	> > the effect would be _exactly_ the same,...
	> 
	> I do not think so.

	Okay.

	Note that we still have to check for dead children in 
	check_max_connections(), and since child_handler() knows nothing 
	about that, it still will write to the pipe, waking up the loop 
	unnecessarily.

	But that will be rare.

	This is no longer as trivial as I wanted it to be, so I'd 
	appreciate a few eyeballs on this patch.

 daemon.c |   69 +++++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/daemon.c b/daemon.c
index 63cd12c..620a288 100644
--- a/daemon.c
+++ b/daemon.c
@@ -16,6 +16,7 @@
 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"
@@ -694,23 +695,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 +785,11 @@ 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);
-			}
+			write(child_handler_pipe[1], &status, 1);
 			continue;
 		}
 		break;
@@ -917,19 +935,24 @@ static int service_loop(int socknum, int *socklist)
 	struct pollfd *pfd;
 	int i;
 
-	pfd = xcalloc(socknum, sizeof(struct pollfd));
+	if (pipe(child_handler_pipe) < 0)
+		die ("Could not set up pipe for child handler");
+
+	pfd = xcalloc(socknum + 1, sizeof(struct pollfd));
 
 	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;
 
-		if (poll(pfd, socknum, -1) < 0) {
+		if (poll(pfd, socknum + 1, -1) < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));
@@ -937,6 +960,10 @@ static int service_loop(int socknum, int *socklist)
 			}
 			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) {
-- 
1.5.6.1.300.gca3f

Re: [PATCH v2] git daemon: avoid calling syslog() from a signal handler

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	Note that we still have to check for dead children in 
> 	check_max_connections(), and since child_handler() knows nothing 
> 	about that, it still will write to the pipe, waking up the loop 
> 	unnecessarily.
>
> 	But that will be rare.
>
> 	This is no longer as trivial as I wanted it to be, so I'd 
> 	appreciate a few eyeballs on this patch.

Yeah, I think the fix-up I sent on top of your patch to make 1-sec timeout
conditional would be more appropriate for 'maint' than this one, even
though it is a band-aid to the issue.

Another thing we might want to consider to make this logic much more
simpler would be to move everything out of child_handler(), except the
write() whose sole purpose is to allow us break out of the poll().

Then you do not have to use "negative is error and positive is success"
convention (which you may regret later when you would need to pass more
than one bit of information from the callsite of waitpid() to the callsite
of syslog()), nor separate "reap here, report there" code structure.

Re: [PATCH v2] git daemon: avoid calling syslog() from a signal handler

[Stephen R. van den Berg]

Junio C Hamano wrote:
>Another thing we might want to consider to make this logic much more
>simpler would be to move everything out of child_handler(), except the
>write() whose sole purpose is to allow us break out of the poll().

As a general rule, keep the work done in a signal handler down to the
bare minimum (like setting an integer flag, and perhaps unblocking the
main thread through a write to this signallingpipe).
-- 
Sincerely,
           Stephen R. van den Berg.

A truly wise man never plays leapfrog with a unicorn.