summaryrefslogtreecommitdiff
path: root/run-command.c
diff options
context:
space:
mode:
authorTakashi Iwai <tiwai@suse.de>2015-09-04 11:35:57 +0200
committerJunio C Hamano <gitster@pobox.com>2015-09-04 14:57:51 -0700
commit507d7804c0b094889cd20f23ad9a48e2b76791f3 (patch)
treeb84dec20adb88cbaaceade47df81dc30f90fd50a /run-command.c
parenta17c56c056d5fea0843b429132904c429a900229 (diff)
downloadgit-507d7804c0b094889cd20f23ad9a48e2b76791f3.tar.gz
pager: don't use unsafe functions in signal handlersti/glibc-stdio-mutex-from-signal-handler
Since the commit a3da8821208d (pager: do wait_for_pager on signal death), we call wait_for_pager() in the pager's signal handler. The recent bug report revealed that this causes a deadlock in glibc at aborting "git log" [*1*]. When this happens, git process is left unterminated, and it can't be killed by SIGTERM but only by SIGKILL. The problem is that wait_for_pager() function does more than waiting for pager process's termination, but it does cleanups and printing errors. Unfortunately, the functions that may be used in a signal handler are very limited [*2*]. Particularly, malloc(), free() and the variants can't be used in a signal handler because they take a mutex internally in glibc. This was the cause of the deadlock above. Other than the direct calls of malloc/free, many functions calling malloc/free can't be used. strerror() is such one, either. Also the usage of fflush() and printf() in a signal handler is bad, although it seems working so far. In a safer side, we should avoid them, too. This patch tries to reduce the calls of such functions in signal handlers. wait_for_signal() takes a flag and avoids the unsafe calls. Also, finish_command_in_signal() is introduced for the same reason. There the free() calls are removed, and only waits for the children without whining at errors. [*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297 [*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'run-command.c')
-rw-r--r--run-command.c25
1 files changed, 17 insertions, 8 deletions
diff --git a/run-command.c b/run-command.c
index 4d73e90fad..fe116bc2b1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -18,26 +18,27 @@ struct child_to_clean {
static struct child_to_clean *children_to_clean;
static int installed_child_cleanup_handler;
-static void cleanup_children(int sig)
+static void cleanup_children(int sig, int in_signal)
{
while (children_to_clean) {
struct child_to_clean *p = children_to_clean;
children_to_clean = p->next;
kill(p->pid, sig);
- free(p);
+ if (!in_signal)
+ free(p);
}
}
static void cleanup_children_on_signal(int sig)
{
- cleanup_children(sig);
+ cleanup_children(sig, 1);
sigchain_pop(sig);
raise(sig);
}
static void cleanup_children_on_exit(void)
{
- cleanup_children(SIGTERM);
+ cleanup_children(SIGTERM, 0);
}
static void mark_child_for_cleanup(pid_t pid)
@@ -232,7 +233,7 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
}
-static int wait_or_whine(pid_t pid, const char *argv0)
+static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
{
int status, code = -1;
pid_t waiting;
@@ -240,6 +241,8 @@ static int wait_or_whine(pid_t pid, const char *argv0)
while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
; /* nothing */
+ if (in_signal)
+ return 0;
if (waiting < 0) {
failed_errno = errno;
@@ -450,7 +453,7 @@ fail_pipe:
* At this point we know that fork() succeeded, but execvp()
* failed. Errors have been reported to our stderr.
*/
- wait_or_whine(cmd->pid, cmd->argv[0]);
+ wait_or_whine(cmd->pid, cmd->argv[0], 0);
failed_errno = errno;
cmd->pid = -1;
}
@@ -549,12 +552,18 @@ fail_pipe:
int finish_command(struct child_process *cmd)
{
- int ret = wait_or_whine(cmd->pid, cmd->argv[0]);
+ int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
argv_array_clear(&cmd->args);
argv_array_clear(&cmd->env_array);
return ret;
}
+int finish_command_in_signal(struct child_process *cmd)
+{
+ return wait_or_whine(cmd->pid, cmd->argv[0], 1);
+}
+
+
int run_command(struct child_process *cmd)
{
int code;
@@ -785,7 +794,7 @@ error:
int finish_async(struct async *async)
{
#ifdef NO_PTHREADS
- return wait_or_whine(async->pid, "child process");
+ return wait_or_whine(async->pid, "child process", 0);
#else
void *ret = (void *)(intptr_t)(-1);