On Thu, Jan 05, 2012 at 05:06:15PM +0100, Clemens Buchacher wrote: > On Wed, Jan 04, 2012 at 09:55:59PM -0500, Jeff King wrote: > > > > It so happens that I have just the patch you need. I've been meaning to > > go over it again and submit it: > > > > run-command: optionally kill children on exit > > https://github.com/peff/git/commit/5523d7ebf2a0386c9c61d7bfbc21375041df4989 > > Thanks, looks great. But if I add this on top (to enable this for > "git daemon"), then t0001 kills my entire X session. Not sure yet > what's going. Yikes. Thanks for noticing. What happens is we have a failure case in start_command, set pid to -1, and then fall through to the end of the function. So we end up marking "-1" for cleanup, which attempts to kill all processes. I never noticed it because it can only happen when fork() fails, or when a child process signals an exec failure (which happens all the time with aliases, but could not be triggered until your patch). The fix is to move the recording of the PID up to a spot where we are certain that it's a real PID. Fixup patch is below, and I'll push a new version out to my github repo. -Peff diff --git a/run-command.c b/run-command.c index aeb9c6e..614b722 100644 --- a/run-command.c +++ b/run-command.c @@ -353,6 +353,8 @@ fail_pipe: if (cmd->pid < 0) error("cannot fork() for %s: %s", cmd->argv[0], strerror(failed_errno = errno)); + else if (cmd->clean_on_exit) + mark_child_for_cleanup(cmd->pid); /* * Wait for child's execvp. If the execvp succeeds (or if fork() @@ -374,8 +376,6 @@ fail_pipe: } close(notify_pipe[0]); - if (cmd->clean_on_exit) - mark_child_for_cleanup(cmd->pid); } #else { -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html