Merge branch 'ti/glibc-stdio-mutex-from-signal-handler'

Allocation related functions and stdio are unsafe things to call
inside a signal handler, and indeed killing the pager can cause
glibc to deadlock waiting on allocation mutex as our signal handler
tries to free() some data structures in wait_for_pager().  Reduce
these unsafe calls.

* ti/glibc-stdio-mutex-from-signal-handler:
  pager: don't use unsafe functions in signal handlers
This commit is contained in:
Junio C Hamano 2015-10-07 13:38:16 -07:00
commit 2b72dbbcf3
3 changed files with 34 additions and 14 deletions

16
pager.c
View File

@ -14,19 +14,29 @@
static const char *pager_argv[] = { NULL, NULL }; static const char *pager_argv[] = { NULL, NULL };
static struct child_process pager_process = CHILD_PROCESS_INIT; static struct child_process pager_process = CHILD_PROCESS_INIT;
static void wait_for_pager(void) static void wait_for_pager(int in_signal)
{ {
if (!in_signal) {
fflush(stdout); fflush(stdout);
fflush(stderr); fflush(stderr);
}
/* signal EOF to pager */ /* signal EOF to pager */
close(1); close(1);
close(2); close(2);
if (in_signal)
finish_command_in_signal(&pager_process);
else
finish_command(&pager_process); finish_command(&pager_process);
} }
static void wait_for_pager_atexit(void)
{
wait_for_pager(0);
}
static void wait_for_pager_signal(int signo) static void wait_for_pager_signal(int signo)
{ {
wait_for_pager(); wait_for_pager(1);
sigchain_pop(signo); sigchain_pop(signo);
raise(signo); raise(signo);
} }
@ -90,7 +100,7 @@ void setup_pager(void)
/* this makes sure that the parent terminates after the pager */ /* this makes sure that the parent terminates after the pager */
sigchain_push_common(wait_for_pager_signal); sigchain_push_common(wait_for_pager_signal);
atexit(wait_for_pager); atexit(wait_for_pager_atexit);
} }
int pager_in_use(void) int pager_in_use(void)

View File

@ -18,26 +18,27 @@ struct child_to_clean {
static struct child_to_clean *children_to_clean; static struct child_to_clean *children_to_clean;
static int installed_child_cleanup_handler; 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) { while (children_to_clean) {
struct child_to_clean *p = children_to_clean; struct child_to_clean *p = children_to_clean;
children_to_clean = p->next; children_to_clean = p->next;
kill(p->pid, sig); kill(p->pid, sig);
if (!in_signal)
free(p); free(p);
} }
} }
static void cleanup_children_on_signal(int sig) static void cleanup_children_on_signal(int sig)
{ {
cleanup_children(sig); cleanup_children(sig, 1);
sigchain_pop(sig); sigchain_pop(sig);
raise(sig); raise(sig);
} }
static void cleanup_children_on_exit(void) static void cleanup_children_on_exit(void)
{ {
cleanup_children(SIGTERM); cleanup_children(SIGTERM, 0);
} }
static void mark_child_for_cleanup(pid_t pid) static void mark_child_for_cleanup(pid_t pid)
@ -220,7 +221,7 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC); 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; int status, code = -1;
pid_t waiting; pid_t waiting;
@ -228,6 +229,8 @@ static int wait_or_whine(pid_t pid, const char *argv0)
while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR) while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
; /* nothing */ ; /* nothing */
if (in_signal)
return 0;
if (waiting < 0) { if (waiting < 0) {
failed_errno = errno; failed_errno = errno;
@ -437,7 +440,7 @@ fail_pipe:
* At this point we know that fork() succeeded, but execvp() * At this point we know that fork() succeeded, but execvp()
* failed. Errors have been reported to our stderr. * 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; failed_errno = errno;
cmd->pid = -1; cmd->pid = -1;
} }
@ -536,12 +539,18 @@ fail_pipe:
int finish_command(struct child_process *cmd) 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->args);
argv_array_clear(&cmd->env_array); argv_array_clear(&cmd->env_array);
return ret; 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 run_command(struct child_process *cmd)
{ {
int code; int code;
@ -786,7 +795,7 @@ error:
int finish_async(struct async *async) int finish_async(struct async *async)
{ {
#ifdef NO_PTHREADS #ifdef NO_PTHREADS
return wait_or_whine(async->pid, "child process"); return wait_or_whine(async->pid, "child process", 0);
#else #else
void *ret = (void *)(intptr_t)(-1); void *ret = (void *)(intptr_t)(-1);

View File

@ -50,6 +50,7 @@ void child_process_init(struct child_process *);
int start_command(struct child_process *); int start_command(struct child_process *);
int finish_command(struct child_process *); int finish_command(struct child_process *);
int finish_command_in_signal(struct child_process *);
int run_command(struct child_process *); int run_command(struct child_process *);
/* /*