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 <peff@peff.net> Signed-off-by: Clemens Buchacher <drizzd@aon.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 7, 2012 at 12:42 UTC afe19ff7b55129d988e421ae1e0df4ec9659787a
2 files changed +69
run-command.c
+68
@@ -1,8 +1,66 @@
1 #include "cache.h"
2 #include "run-command.h"
3 #include "exec_cmd.h"
4 +#include "sigchain.h"
5 #include "argv-array.h"
6
7 +struct child_to_clean {
8 + pid_t pid;
9 + struct child_to_clean *next;
10 +};
11 +static struct child_to_clean *children_to_clean;
12 +static int installed_child_cleanup_handler;
13 +
14 +static void cleanup_children(int sig)
15 +{
16 + while (children_to_clean) {
17 + struct child_to_clean *p = children_to_clean;
18 + children_to_clean = p->next;
19 + kill(p->pid, sig);
20 + free(p);
21 + }
22 +}
23 +
24 +static void cleanup_children_on_signal(int sig)
25 +{
26 + cleanup_children(sig);
27 + sigchain_pop(sig);
28 + raise(sig);
29 +}
30 +
31 +static void cleanup_children_on_exit(void)
32 +{
33 + cleanup_children(SIGTERM);
34 +}
35 +
36 +static void mark_child_for_cleanup(pid_t pid)
37 +{
38 + struct child_to_clean *p = xmalloc(sizeof(*p));
39 + p->pid = pid;
40 + p->next = children_to_clean;
41 + children_to_clean = p;
42 +
43 + if (!installed_child_cleanup_handler) {
44 + atexit(cleanup_children_on_exit);
45 + sigchain_push_common(cleanup_children_on_signal);
46 + installed_child_cleanup_handler = 1;
47 + }
48 +}
49 +
50 +static void clear_child_for_cleanup(pid_t pid)
51 +{
52 + struct child_to_clean **last, *p;
53 +
54 + last = &children_to_clean;
55 + for (p = children_to_clean; p; p = p->next) {
56 + if (p->pid == pid) {
57 + *last = p->next;
58 + free(p);
59 + return;
60 + }
61 + }
62 +}
63 +
64 static inline void close_pair(int fd[2])
65 {
66 close(fd[0]);
@@ -130,6 +188,9 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
188 } else {
189 error("waitpid is confused (%s)", argv0);
190 }
191 +
192 + clear_child_for_cleanup(pid);
193 +
194 errno = failed_errno;
195 return code;
196 }
@@ -292,6 +353,8 @@ fail_pipe:
353 if (cmd->pid < 0)
354 error("cannot fork() for %s: %s", cmd->argv[0],
355 strerror(failed_errno = errno));
356 + else if (cmd->clean_on_exit)
357 + mark_child_for_cleanup(cmd->pid);
358
359 /*
360 * Wait for child's execvp. If the execvp succeeds (or if fork()
@@ -312,6 +375,7 @@ fail_pipe:
375 cmd->pid = -1;
376 }
377 close(notify_pipe[0]);
378 +
379 }
380 #else
381 {
@@ -356,6 +420,8 @@ fail_pipe:
420 failed_errno = errno;
421 if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
422 error("cannot spawn %s: %s", cmd->argv[0], strerror(errno));
423 + if (cmd->clean_on_exit && cmd->pid >= 0)
424 + mark_child_for_cleanup(cmd->pid);
425
426 if (cmd->env)
427 free_environ(env);
@@ -540,6 +606,8 @@ int start_async(struct async *async)
606 exit(!!async->proc(proc_in, proc_out, async->data));
607 }
608
609 + mark_child_for_cleanup(async->pid);
610 +
611 if (need_in)
612 close(fdin[0]);
613 else if (async->in)
run-command.h
+1
@@ -38,6 +38,7 @@ struct child_process {
38 unsigned silent_exec_failure:1;
39 unsigned stdout_to_stderr:1;
40 unsigned use_shell:1;
41 + unsigned clean_on_exit:1;
42 void (*preexec_cb)(void);
43 };
44