From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> In order to avoid invoking a signal handler in the child process between fork() and exec(), start_command() blocks all signals before calling fork and then in the child resets the disposition of all signals that are not ignored to SIG_DFL before unblocking them. The exception for ignored signals seems to been inspired by ruby's process handling[1] based on the misconception that execve() will reset them to SIG_DFL. Unfortunately this is not the case [2] and any signals that are ignored in the parent will default to being ignored by the program executed by start_command(). When git ignores SIGPIPE before forking a child process it is to stop git from being killed if the child exits while git is writing to the child's stdin. We do not want to ignore SIGPIPE in the child. When git ignores SIGINT and SIGQUIT before forking a child process it is to stop git from being killed if the user interrupts the child with Ctrl-C or Ctrl-\ we do not want the child to ignore those signals [3]. Fortunately the fix is easy - reset the disposition of all signals regardless of their previous disposition. [1] https://lore.kernel.org/git/20170413211428.GA5961@whir/ [2] The man page for execve(2) states: POSIX.1 specifies that the dispositions of any signals that are ignored or set to the default are left unchanged. POSIX.1 specifies one exception: if SIGCHLD is being ignored, then an implementation may leave the disposition unchanged or reset it to the default; Linux does the former. Page 579 of "The Linux Programming Interface" notes: SUSv3 recommends that signals should not be blokced or ignored across an exec() of an arbitrary program. Here, "arbitrary" means a program that we did not write. [3] This is really a work-around for not moving the child into its own process group and changing the foreground process group of the controlling terminal. Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> --- start_command: reset disposition of all signals in child As an aside I wonder if we ought to add an option to ignore SIGPIPE when stdin is redirected and possibly turn it on by default. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1582%2Fphillipwood%2Fstart-command-dont-ignore-signals-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1582/phillipwood/start-command-dont-ignore-signals-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1582 run-command.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/run-command.c b/run-command.c index a558042c876..765775a1f42 100644 --- a/run-command.c +++ b/run-command.c @@ -823,11 +823,8 @@ fail_pipe: * restore default signal handlers here, in case * we catch a signal right before execve below */ - for (sig = 1; sig < NSIG; sig++) { - /* ignored signals get reset to SIG_DFL on execve */ - if (signal(sig, SIG_DFL) == SIG_IGN) - signal(sig, SIG_IGN); - } + for (sig = 1; sig < NSIG; sig++) + signal(sig, SIG_DFL); if (sigprocmask(SIG_SETMASK, &as.old, NULL) != 0) child_die(CHILD_ERR_SIGPROCMASK); base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3 -- gitgitgadget
