Ignore SIGINT and SIGQUIT in non-interactive background processes

CHANGELOG.md

@@ -5,6 +5,7 @@
 - `jobs --quiet PID` will no longer print 'no suitable job' if the job for PID does not exist (e.g. because it has finished). #6809
 - A variable `fish_kill_signal` will be set to the signal that terminated the last foreground job, or `0` if the job exited normally.
 - On BSD systems, with the `-s` option, `fish_md5` does not use the given string, but `-s`. From now on the string is used.
+- Control-C no longer kills background jobs for which job control is disabled, matching POSIX semantics (#6828).
 
 ### Syntax changes and new commands
 

src/exec.cpp

@@ -187,7 +187,7 @@ static void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &block_i
 
     // child_setup_process makes sure signals are properly set up.
     dup2_list_t redirs = dup2_list_t::resolve_chain(all_ios);
-    if (child_setup_process(INVALID_PID, false, redirs) == 0) {
+    if (child_setup_process(INVALID_PID, *j, false, redirs) == 0) {
         // Decrement SHLVL as we're removing ourselves from the shell "stack".
         auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT);
         wcstring shlvl_str = L"0";
@@ -325,6 +325,16 @@ static void run_internal_process_or_short_circuit(parser_t &parser, const std::s
     }
 }
 
+bool blocked_signals_for_job(const job_t &job, sigset_t *sigmask) {
+    // Block some signals in background jobs for which job control is turned off (#6828).
+    if (!job.is_foreground() && !job.wants_job_control()) {
+        sigaddset(sigmask, SIGINT);
+        sigaddset(sigmask, SIGQUIT);
+        return true;
+    }
+    return false;
+}
+
 /// Call fork() as part of executing a process \p p in a job \j. Execute \p child_action in the
 /// context of the child. Returns true if fork succeeded, false if fork failed.
 static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t *p,
@@ -337,7 +347,7 @@ static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t
         maybe_t<pid_t> new_termowner{};
         p->pid = getpid();
         child_set_group(job.get(), p);
-        child_setup_process(job->should_claim_terminal() ? job->pgid : INVALID_PID, true, dup2s);
+        child_setup_process(job->should_claim_terminal() ? job->pgid : INVALID_PID, *job, true, dup2s);
         child_action();
         DIE("Child process returned control to fork_child lambda!");
     }

src/exec.h

@@ -39,4 +39,8 @@ void exec_close(int fd);
 /// assigned. It's factored out because the logic has subtleties, and this centralizes it.
 pgroup_provenance_t get_pgroup_provenance(const std::shared_ptr<job_t> &j,
                                           const job_lineage_t &lineage);
+
+/// Add signals that should be masked for external processes in this job.
+bool blocked_signals_for_job(const job_t &job, sigset_t *sigmask);
+
 #endif

src/fish_test_helper.cpp

@@ -76,11 +76,16 @@ static void print_blocked_signals() {
         perror("sigprocmask");
         exit(EXIT_FAILURE);
     }
-    // There is no obviously portable way to get the maximum number of signals; here we limit it to 128.
-    for (int sig=1; sig < 128; sig++) {
+    // There is no obviously portable way to get the maximum number of signals.
+    // Here we limit it to 64 because strsignal on Linux returns "Unknown signal" for anything above.
+    for (int sig=1; sig < 65; sig++) {
         if (sigismember(&sigs, sig)) {
             if (const char *s = strsignal(sig)) {
-                fprintf(stderr, "%s\n", s);
+                fprintf(stderr, "%s", s);
+                if (strchr(s, ':') == nullptr) {
+                    fprintf(stderr, ": %d", sig);
+                }
+                fprintf(stderr, "\n");
             }
         }
     }

src/postfork.cpp

@@ -135,7 +135,7 @@ bool set_child_group(job_t *j, pid_t child_pid) {
     return true;
 }
 
-int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &dup2s) {
+int child_setup_process(pid_t new_termowner, const job_t &job, bool is_forked, const dup2_list_t &dup2s) {
     // Note we are called in a forked child.
     for (const auto &act : dup2s.get_actions()) {
         int err;
@@ -170,6 +170,11 @@ int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &
             (void)tcsetpgrp(STDIN_FILENO, new_termowner);
         }
     }
+    sigset_t sigmask;
+    sigemptyset(&sigmask);
+    if (blocked_signals_for_job(job, &sigmask)) {
+        sigprocmask(SIG_SETMASK, &sigmask, nullptr);
+    }
     // Set the handling for job control signals back to the default.
     // Do this after any tcsetpgrp call so that we swallow SIGTTIN.
     signal_reset_handlers();
@@ -275,7 +280,10 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr,
     // No signals blocked.
     sigset_t sigmask;
     sigemptyset(&sigmask);
-    if (!err && reset_sigmask) err = posix_spawnattr_setsigmask(attr, &sigmask);
+    if (!err && reset_sigmask) {
+        blocked_signals_for_job(*j, &sigmask);
+        err = posix_spawnattr_setsigmask(attr, &sigmask);
+    }
 
     // Apply our dup2s.
     for (const auto &act : dup2s.get_actions()) {

src/postfork.h

@@ -31,7 +31,7 @@ bool child_set_group(job_t *j, process_t *p);     // called by child
 ///
 /// \return 0 on success, -1 on failure. When this function returns, signals are always unblocked.
 /// On failure, signal handlers, io redirections and process group of the process is undefined.
-int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &dup2s);
+int child_setup_process(pid_t new_termowner, const job_t &job, bool is_forked, const dup2_list_t &dup2s);
 
 /// Call fork(), retrying on failure a few times.
 pid_t execute_fork();

tests/checks/sigint.fish

@@ -1,5 +1,17 @@
 #RUN: %fish -C "set helper %fish_test_helper" %s
 
+# Block some signals if job control is off (#6828).
+
+status job-control none
+for fish_use_posix_spawn in 0 1
+	$helper print_blocked_signals &
+	wait
+end
+# CHECKERR: Interrupt: 2
+# CHECKERR: Quit: 3
+# CHECKERR: Interrupt: 2
+# CHECKERR: Quit: 3
+
 # Ensure we can break from a while loop.
 
 echo About to sigint

tests/signals.expect

@@ -2,9 +2,18 @@
 #
 # Test signal handling for interactive shells.
 
-set pid [spawn $fish]
+set pid [spawn $fish -C "sleep 10&"]
 expect_prompt
 
+send "\x03"
+sleep 0.010
+send_line "jobs"
+expect_prompt -re {sleep.10} {} unmatched {
+    puts stderr "background job missing after SIGINT"
+}
+send_line "kill %1"
+expect_prompt 
+
 # Verify that the fish_postexec handler is called after SIGINT.
 send_line "function postexec --on-event fish_postexec; echo fish_postexec spotted; end"
 expect_prompt