Re: [PATCH 1/2] daemon: add tests

[Clemens Buchacher <drizzd@aon.at>]

On Wed, Jan 04, 2012 at 05:26:49PM -0500, Jeff King wrote:
> 
> Or is the problem the git wrapper itself, which doesn't kill its
> subprocess when it dies (which IMHO is a bug which we might want to
> fix)? In that case, couldn't we just use --pid-file to save the actual
> daemon pid, and then kill using that?

Or like this. Doesn't work with multiple children. I have yet to
check if we have those anywhere.

diff --git a/run-command.c b/run-command.c
index 1c51043..0c105e6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -65,6 +65,22 @@ static int execv_shell_cmd(const char **argv)
 #ifndef WIN32
 static int child_err = 2;
 static int child_notifier = -1;
+static struct child_process *current_cmd;
+
+static void kill_current_cmd(int signo)
+{
+	signal(signo, SIG_DFL);
+
+	if (current_cmd) {
+		if (current_cmd->pid) {
+			/* forward signal to the child process */
+			kill(current_cmd->pid, signo);
+		} else {
+			/* trigger the default signal handler */
+			raise(signo);
+		}
+	}
+}
 
 static void notify_parent(void)
 {
@@ -201,6 +217,9 @@ fail_pipe:
 	if (pipe(notify_pipe))
 		notify_pipe[0] = notify_pipe[1] = -1;
 
+	current_cmd = cmd;
+	signal(SIGTERM, kill_current_cmd);
+
 	cmd->pid = fork();
 	if (!cmd->pid) {
 		/*
diff --git a/t/lib-daemon.sh b/t/lib-daemon.sh
index b2ffd54..c1c41ee 100644
--- a/t/lib-daemon.sh
+++ b/t/lib-daemon.sh
@@ -41,8 +41,8 @@ stop_daemon() {
 
 	# kill git-daemon child of git
 	say >&3 "Stopping git daemon ..."
-	pkill -P "$DAEMON_PID"
-	wait "$DAEMON_PID"
+	kill "$DAEMON_PID"
+	wait "$DAEMON_PID" >&3 2>&4
 	ret=$?
 	#
 	# We signal TERM=15 to the child and expect the parent to

> As a side note, it looks like we just start the daemon with "git daemon
> &". Doesn't that create a race condition with the tests which
> immediately try to access it (i.e., the first test may run before the
> daemon actually opens the socket)?

That's correct. How would I fix that? Try connecting and sleep in a
loop until ready or timeout? Will look into that.