From ec44d98347fe5e5bdcaff77db61df636644a1ee2 Mon Sep 17 00:00:00 2001
From: Johannes Altmanninger <aclopte@gmail.com>
Date: Thu, 7 Mar 2024 14:49:50 +0100
Subject: [PATCH] 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.
---
 src/client.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/client.cc b/src/client.cc
index 20123c2fe..4f32e5d36 100644
--- a/src/client.cc
+++ b/src/client.cc
@@ -49,9 +49,9 @@ Client::Client(std::unique_ptr<UserInterface>&& ui,
         kak_assert(key != Key::Invalid);
         if (key == ctrl('c'))
         {
-            auto prev_handler = set_signal_handler(SIGINT, SIG_IGN);
-            killpg(getpgrp(), SIGINT);
-            set_signal_handler(SIGINT, prev_handler);
+            auto prev_handler = set_signal_handler(SIGTERM, SIG_IGN);
+            killpg(getpgrp(), SIGTERM);
+            set_signal_handler(SIGTERM, prev_handler);
         }
         else if (key == ctrl('g'))
         {