path: root/pager.c
diff options
authorTakashi Iwai <>2015-09-04 09:35:57 (GMT)
committerJunio C Hamano <>2015-09-04 21:57:51 (GMT)
commit507d7804c0b094889cd20f23ad9a48e2b76791f3 (patch)
treeb84dec20adb88cbaaceade47df81dc30f90fd50a /pager.c
parenta17c56c056d5fea0843b429132904c429a900229 (diff)
pager: don't use unsafe functions in signal handlers
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*] [*2*] Signed-off-by: Takashi Iwai <> Reviewed-by: Jeff King <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'pager.c')
1 files changed, 16 insertions, 6 deletions
diff --git a/pager.c b/pager.c
index 070dc11..0f789c3 100644
--- a/pager.c
+++ b/pager.c
@@ -14,19 +14,29 @@
static const char *pager_argv[] = { NULL, NULL };
static struct child_process pager_process = CHILD_PROCESS_INIT;
-static void wait_for_pager(void)
+static void wait_for_pager(int in_signal)
- fflush(stdout);
- fflush(stderr);
+ if (!in_signal) {
+ fflush(stdout);
+ fflush(stderr);
+ }
/* signal EOF to pager */
- finish_command(&pager_process);
+ if (in_signal)
+ finish_command_in_signal(&pager_process);
+ else
+ finish_command(&pager_process);
+static void wait_for_pager_atexit(void)
+ wait_for_pager(0);
static void wait_for_pager_signal(int signo)
- wait_for_pager();
+ wait_for_pager(1);
@@ -90,7 +100,7 @@ void setup_pager(void)
/* this makes sure that the parent terminates after the pager */
- atexit(wait_for_pager);
+ atexit(wait_for_pager_atexit);
int pager_in_use(void)