Allow trapping SIGINT and SIGTERM in scripts

This teaches `--on-signal SIGINT` (and by extension `trap cmd SIGINT`)
to work properly in scripts, not just interactively. Note any such
function will suppress the default behavior of exiting. Do this for
SIGTERM as well.

CHANGELOG.rst

@@ -73,6 +73,7 @@ Scripting improvements
 - ``string join`` gained a new ``--no-empty`` flag to skip empty arguments (:issue:`8774`, :issue:`8351`).
 - ``read`` now actually only triggers the ``fish_read`` event, not the ``fish_prompt`` event (:issue:`8797`). It was supposed to work this way since fish 3.2.0.
 - The tty modes are no longer restored when non-interactive shells exit. This fixes wrong tty modes in pipelines with interactive commands. (:issue:`8705`).
+- Scripts can now catch and handle SIGINT and SIGTERM, either via ``function --on-signal`` or with ``trap``. (:issue:`6649`).
 
 Interactive improvements
 ------------------------

doc_src/cmds/function.rst

@@ -42,7 +42,7 @@ The following options are available:
     Run this function when the fish child process with process ID PID exits. Instead of a PID, for backward compatibility, "``%self``" can be specified as an alias for ``$fish_pid``, and the function will be run when the current fish instance exits.
 
 **-s** *SIGSPEC* or **--on-signal** *SIGSPEC*
-    Run this function when the signal ``SIGSPEC`` is delivered. ``SIGSPEC`` can be a signal number, or the signal name, such as ``SIGHUP`` (or just ``HUP``). Note that the signal must have been delivered to :program:`fish`; for example, :kbd:`Ctrl-C` sends ``SIGINT`` to the foreground process group, which will not be :program:`fish` if you are running another command at the time.
+    Run this function when the signal ``SIGSPEC`` is delivered. ``SIGSPEC`` can be a signal number, or the signal name, such as ``SIGHUP`` (or just ``HUP``). Note that the signal must have been delivered to :program:`fish`; for example, :kbd:`Ctrl-C` sends ``SIGINT`` to the foreground process group, which will not be :program:`fish` if you are running another command at the time. Observing a signal will prevent fish from exiting in response to that signal.
 
 **-S** or **--no-scope-shadowing**
     Allows the function to access the variables of calling functions. Normally, any variables inside the function that have the same name as variables from the calling function are "shadowed", and their contents are independent of the calling function.

doc_src/cmds/trap.rst

@@ -38,7 +38,7 @@ If *ARG* is absent (and there is a single *REASON*) or ``-``, each specified sig
 
 If *ARG* is not present and **-p** has been supplied, then the trap commands associated with each *REASON* are displayed. If no arguments are supplied or if only **-p** is given, ``trap`` prints the list of commands associated with each signal.
 
-Signal names are case insensitive and the ``SIG`` prefix is optional.
+Signal names are case insensitive and the ``SIG`` prefix is optional. Trapping a signal will prevent fish from exiting in response to that signal.
 
 The exit status is 1 if any *REASON* is invalid; otherwise trap returns 0.
 

src/proc.cpp

@@ -225,14 +225,19 @@ static void handle_child_status(const shared_ptr<job_t> &job, process_t *proc,
         proc->completed = true;
     }
 
-    // If the child was killed by SIGINT or SIGQUIT, then treat it as if we received that signal.
+    // If the child was killed by SIGINT or SIGQUIT, then cancel the entire group if interactive. If
+    // not interactive, we have historically re-sent the signal to ourselves; however don't do that
+    // if the signal is trapped (#6649).
+    // Note the asymmetry: if the fish process gets SIGINT we will run SIGINT handlers. If a child
+    // process gets SIGINT we do not run SIGINT handlers; we just don't exit. This should be
+    // rationalized.
     if (status.signal_exited()) {
         int sig = status.signal_code();
         if (sig == SIGINT || sig == SIGQUIT) {
             if (is_interactive_session()) {
                 // Mark the job group as cancelled.
                 job->group->cancel_with_signal(sig);
-            } else {
+            } else if (!event_is_signal_observed(sig)) {
                 // Deliver the SIGINT or SIGQUIT signal to ourself since we're not interactive.
                 struct sigaction act;
                 sigemptyset(&act.sa_mask);

src/signal.cpp

@@ -233,14 +233,13 @@ static void fish_signal_handler(int sig, siginfo_t *info, void *context) {
     switch (sig) {
 #ifdef SIGWINCH
         case SIGWINCH:
-            /// Respond to a winch signal by telling the termsize container.
+            // Respond to a winch signal by telling the termsize container.
             termsize_container_t::handle_winch();
             break;
 #endif
 
         case SIGHUP:
-            /// Respond to a hup signal by exiting, unless it is caught by a shellscript function,
-            /// in which case we do nothing.
+            // Exit unless the signal was trapped.
             if (!observed) {
                 reader_sighup();
             }
@@ -248,16 +247,19 @@ static void fish_signal_handler(int sig, siginfo_t *info, void *context) {
             break;
 
         case SIGTERM:
-            /// Handle sigterm. The only thing we do is restore the front process ID, then die.
-            restore_term_foreground_process_group_for_exit();
-            signal(SIGTERM, SIG_DFL);
-            raise(SIGTERM);
+            // Handle sigterm. The only thing we do is restore the front process ID, then die.
+            if (!observed) {
+                restore_term_foreground_process_group_for_exit();
+                signal(SIGTERM, SIG_DFL);
+                raise(SIGTERM);
+            }
             break;
 
         case SIGINT:
-            /// Interactive mode ^C handler. Respond to int signal by setting interrupted-flag and
-            /// stopping all loops and conditionals.
-            s_cancellation_signal = SIGINT;
+            // Cancel unless the signal was trapped.
+            if (!observed) {
+                s_cancellation_signal = SIGINT;
+            }
             reader_handle_sigint();
             topic_monitor_t::principal().post(topic_t::sighupint);
             break;

tests/checks/trap.fish

@@ -1,4 +1,4 @@
-# RUN: %fish %s
+# RUN: env fth=%fish_test_helper %fish %s
 
 set -g SIGUSR1_COUNT 0
 
@@ -49,3 +49,41 @@ echo "Hope it did not run"
 kill -USR1 $fish_pid
 sleep .1
 #CHECK: Got USR1: 3
+
+# We can trap SIGINT.
+# Trapping it prevents exiting.
+function handle_int --on-signal SIGINT
+    echo Got SIGINT
+end
+kill -INT $fish_pid
+#CHECK: Got SIGINT
+
+# In non-interactive mode, we have historically treated
+# a child process which dies with SIGINT as if we got SIGINT.
+# However in this case we don't execute handlers; we just don't exit.
+$fth sigint_self
+echo "Should not have exited"
+#CHECK: Should not have exited
+
+# If a signal is received while handling a signal, that is deferred until the handler is done.
+set -g INTCOUNT 0
+function handle_int --on-signal SIGINT
+    test $INTCOUNT -gt 2 && return
+    set -g INTCOUNT (math $INTCOUNT + 1)
+    echo "start handle_int $INTCOUNT"
+    sleep .1
+    kill -INT $fish_pid
+    echo "end handle_int $INTCOUNT"
+end
+kill -INT $fish_pid
+# CHECK: start handle_int 1
+# CHECK: end handle_int 1
+# CHECK: start handle_int 2
+# CHECK: end handle_int 2
+# CHECK: start handle_int 3
+# CHECK: end handle_int 3
+
+# Remove our handler and SIGINT ourselves. Now we should exit.
+functions --erase handle_int
+kill -INT $fish_pid
+echo "I should not run"