From afe19ff7b55129d988e421ae1e0df4ec9659787a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 7 Jan 2012 12:42:43 +0100 Subject: [PATCH 1/2] run-command: optionally kill children on exit When we spawn a helper process, it should generally be done and finish_command called before we exit. However, if we exit abnormally due to an early return or a signal, the helper may continue to run in our absence. In the best case, this may simply be wasted CPU cycles or a few stray messages on a terminal. But it could also mean a process that the user thought was aborted continues to run to completion (e.g., a push's pack-objects helper will complete the push, even though you killed the push process). This patch provides infrastructure for run-command to keep track of PIDs to be killed, and clean them on signal reception or input, just as we do with tempfiles. PIDs can be added in two ways: 1. If NO_PTHREADS is defined, async helper processes are automatically marked. By definition this code must be ready to die when the parent dies, since it may be implemented as a thread of the parent process. 2. If the run-command caller specifies the "clean_on_exit" option. This is not the default, as there are cases where it is OK for the child to outlive us (e.g., when spawning a pager). PIDs are cleared from the kill-list automatically during wait_or_whine, which is called from finish_command and finish_async. Signed-off-by: Jeff King Signed-off-by: Clemens Buchacher Signed-off-by: Junio C Hamano --- run-command.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ run-command.h | 1 + 2 files changed, 69 insertions(+) diff --git a/run-command.c b/run-command.c index 1c51043884..0204aaf7e8 100644 --- a/run-command.c +++ b/run-command.c @@ -1,8 +1,66 @@ #include "cache.h" #include "run-command.h" #include "exec_cmd.h" +#include "sigchain.h" #include "argv-array.h" +struct child_to_clean { + pid_t pid; + struct child_to_clean *next; +}; +static struct child_to_clean *children_to_clean; +static int installed_child_cleanup_handler; + +static void cleanup_children(int sig) +{ + while (children_to_clean) { + struct child_to_clean *p = children_to_clean; + children_to_clean = p->next; + kill(p->pid, sig); + free(p); + } +} + +static void cleanup_children_on_signal(int sig) +{ + cleanup_children(sig); + sigchain_pop(sig); + raise(sig); +} + +static void cleanup_children_on_exit(void) +{ + cleanup_children(SIGTERM); +} + +static void mark_child_for_cleanup(pid_t pid) +{ + struct child_to_clean *p = xmalloc(sizeof(*p)); + p->pid = pid; + p->next = children_to_clean; + children_to_clean = p; + + if (!installed_child_cleanup_handler) { + atexit(cleanup_children_on_exit); + sigchain_push_common(cleanup_children_on_signal); + installed_child_cleanup_handler = 1; + } +} + +static void clear_child_for_cleanup(pid_t pid) +{ + struct child_to_clean **last, *p; + + last = &children_to_clean; + for (p = children_to_clean; p; p = p->next) { + if (p->pid == pid) { + *last = p->next; + free(p); + return; + } + } +} + static inline void close_pair(int fd[2]) { close(fd[0]); @@ -130,6 +188,9 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure) } else { error("waitpid is confused (%s)", argv0); } + + clear_child_for_cleanup(pid); + errno = failed_errno; return code; } @@ -292,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() @@ -312,6 +375,7 @@ fail_pipe: cmd->pid = -1; } close(notify_pipe[0]); + } #else { @@ -356,6 +420,8 @@ fail_pipe: failed_errno = errno; if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT)) error("cannot spawn %s: %s", cmd->argv[0], strerror(errno)); + if (cmd->clean_on_exit && cmd->pid >= 0) + mark_child_for_cleanup(cmd->pid); if (cmd->env) free_environ(env); @@ -540,6 +606,8 @@ int start_async(struct async *async) exit(!!async->proc(proc_in, proc_out, async->data)); } + mark_child_for_cleanup(async->pid); + if (need_in) close(fdin[0]); else if (async->in) diff --git a/run-command.h b/run-command.h index 56491b9f23..2a6946668b 100644 --- a/run-command.h +++ b/run-command.h @@ -38,6 +38,7 @@ struct child_process { unsigned silent_exec_failure:1; unsigned stdout_to_stderr:1; unsigned use_shell:1; + unsigned clean_on_exit:1; void (*preexec_cb)(void); }; From 10c6cddd928b24ac6030a172c6c7b46efb32aedc Mon Sep 17 00:00:00 2001 From: Clemens Buchacher Date: Sun, 8 Jan 2012 21:41:09 +0100 Subject: [PATCH 2/2] dashed externals: kill children on exit Several git commands are so-called dashed externals, that is commands executed as a child process of the git wrapper command. If the git wrapper is killed by a signal, the child process will continue to run. This is different from internal commands, which always die with the git wrapper command. Enable the recently introduced cleanup mechanism for child processes in order to make dashed externals act more in line with internal commands. Signed-off-by: Clemens Buchacher Acked-by: Jeff King Signed-off-by: Junio C Hamano --- git.c | 2 +- run-command.c | 1 + run-command.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/git.c b/git.c index fb9029cbf1..3805616630 100644 --- a/git.c +++ b/git.c @@ -495,7 +495,7 @@ static void execv_dashed_external(const char **argv) * if we fail because the command is not found, it is * OK to return. Otherwise, we just pass along the status code. */ - status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE); + status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT); if (status >= 0 || errno != ENOENT) exit(status); diff --git a/run-command.c b/run-command.c index 0204aaf7e8..1db8abf984 100644 --- a/run-command.c +++ b/run-command.c @@ -497,6 +497,7 @@ static void prepare_run_command_v_opt(struct child_process *cmd, cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0; cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0; cmd->use_shell = opt & RUN_USING_SHELL ? 1 : 0; + cmd->clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0; } int run_command_v_opt(const char **argv, int opt) diff --git a/run-command.h b/run-command.h index 2a6946668b..44f7d2bd42 100644 --- a/run-command.h +++ b/run-command.h @@ -53,6 +53,7 @@ extern int run_hook(const char *index_file, const char *name, ...); #define RUN_COMMAND_STDOUT_TO_STDERR 4 #define RUN_SILENT_EXEC_FAILURE 8 #define RUN_USING_SHELL 16 +#define RUN_CLEAN_ON_EXIT 32 int run_command_v_opt(const char **argv, int opt); /*