Send SIGTERM on <c-c>, to more reliably kill background jobs
Consider sh -c 'sleep 5 & sleep inf' Since the shell is non-interactive, there is no job control. This makes the shell spawn the "sleep 5" process in the shell's own process group[1] - presumably, because only interactive shells have a need to forward signals to all processes in its foreground job. When this non-interactive shell process is cancelled with SIGINT, "sleep 5" keeps running [2]. At least the dash shell implements this by running "signal(SIGINT, SIG_IGN)" in the forked child. Unless the child process explicitly overrides that (to SIG_DFL for example), it will ignore SIGINT. Probably the reason for this behavior is to feign consistency with interactive shells, without needing to actually run background jobs in a dedicated process group like interactive shells do. Bash documents this behavior[3]: > When job control is not in effect, asynchronous commands ignore > SIGINT and SIGQUIT in addition to these inherited handlers. Several of our scripts[4] - most prominently ":make" - use the "</dev/null >/dev/null 2>&1 &" pattern to run potentially long-running processes in the background, without blocking the editor. On <c-c>, we send SIGINT to our process group. As explained above, this will generally not terminate any background processes. This problem has been masked by a behavior that is unique to using both Bash and its "eval" builtin. Given nop %sh{ rm -f /tmp/fifo mkfifo /tmp/fifo ( eval make >/tmp/fifo 2>&1 & ) >/dev/null 2>&1 </dev/null } edit -fifo /tmp/fifo *my-fifo* When running this and pressing Control+C, Bash actually terminates the Make processes. However if I remove the "eval", it no longer does. This doesn't seems like something we should rely on. Other commands like ":git blame" don't use "eval" so they cannot be cancelled today. Fix these issues by sending SIGTERM instead of SIGINT, which should apply to the whole process group with pretty much the same effect. Barely tested, let's see if this breaks some weird build system. In future we might allow more fine-grained control over which processes are cancelled by <c-c>. {{{ Alternative solution: With the above fix, scripts can opt-out of being terminated by <c-c> by using setsid (though that's not POSIX unfortunately, and may require nesting quotes) or the classic Unix double-forking trick to create a daemon process. Though it is certainly possible that someone expects commands like this to survive <c-c>: nop %sh{ tail -f my-log </dev/null 2>&1 | grep some-error > some-file 2>&1 & } I think it would be ideal to stick to SIGINT and match semantics of a noninteractive shell, to avoid muddying the waters. Background processes could still **opt into** being terminated by <c-c>. For example by providing a simple program in libexec/ that does // interruptible.c int main(int argc, char** argv) { signal(SIGINT, SIG_DFL); execv(argv[1], &argv[1]); } used as diff --git a/rc/tools/make.kak b/rc/tools/make.kak index b88f7e538..f6e041908 100644 --- a/rc/tools/make.kak +++ b/rc/tools/make.kak @@ -16,3 +16,3 @@ define-command -params .. \ mkfifo ${output} - ( eval "${kak_opt_makecmd}" "$@" > ${output} 2>&1 & ) > /dev/null 2>&1 < /dev/null + ( eval "interruptible ${kak_opt_makecmd}" "$@" > ${output} 2>&1 & ) > /dev/null 2>&1 < /dev/null Unfortunately, it's inconvenient to add "interruptible" to commands like clang-parse and git-blame because they background a whole subshell with many commands, so we'd need to nest quotes. Also I'm not sure if this brings any benefit. So I didn't explore this further yet although we can definitely do that. }}} Fixes #3751 [1]: https://stackoverflow.com/questions/45106725/why-do-shells-ignore-sigint-and-sigquit-in-backgrounded-processes/45106961#45106961 [2]: https://unix.stackexchange.com/questions/372541/why-doesnt-sigint-work-on-a-background-process-in-a-script/677742#677742 [3]: https://www.gnu.org/software/bash/manual/html_node/Signals.html [4]: clang-parse, ctags-*, git blame, git log, gopls references, grep, jedi-complete, lint-*, make; I don't think any of these should be uninterruptible.
This commit is contained in:
parent
f26d4ea4bf
commit
ec44d98347
|
@ -49,9 +49,9 @@ Client::Client(std::unique_ptr<UserInterface>&& ui,
|
||||||
kak_assert(key != Key::Invalid);
|
kak_assert(key != Key::Invalid);
|
||||||
if (key == ctrl('c'))
|
if (key == ctrl('c'))
|
||||||
{
|
{
|
||||||
auto prev_handler = set_signal_handler(SIGINT, SIG_IGN);
|
auto prev_handler = set_signal_handler(SIGTERM, SIG_IGN);
|
||||||
killpg(getpgrp(), SIGINT);
|
killpg(getpgrp(), SIGTERM);
|
||||||
set_signal_handler(SIGINT, prev_handler);
|
set_signal_handler(SIGTERM, prev_handler);
|
||||||
}
|
}
|
||||||
else if (key == ctrl('g'))
|
else if (key == ctrl('g'))
|
||||||
{
|
{
|
||||||
|
|
Loading…
Reference in New Issue
Block a user