From 77d33a8eb92f179c8c2b08f6c09ed5004c90c053 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 5 Apr 2020 10:24:26 +0200 Subject: [PATCH] Ignore SIGINT and SIGQUIT in non-interactive background processes Fixes #6828 --- CHANGELOG.md | 1 + src/exec.cpp | 14 ++++++++++++-- src/exec.h | 4 ++++ src/postfork.cpp | 12 ++++++++++-- src/postfork.h | 2 +- tests/checks/sigint.fish | 12 ++++++++++++ tests/signals.expect | 11 ++++++++++- 7 files changed, 50 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be0e07e9d..20f8b71d0 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/exec.cpp b/src/exec.cpp index c8d58a5d7..bcffd6ef8 100644 --- a/src/exec.cpp +++ b/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, process_t *p, @@ -337,7 +347,7 @@ static bool fork_child_for_process(const std::shared_ptr &job, process_t maybe_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!"); } diff --git a/src/exec.h b/src/exec.h index 640b65bbf..6c52be8db 100644 --- a/src/exec.h +++ b/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 &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 diff --git a/src/postfork.cpp b/src/postfork.cpp index 459b9349b..1fbf1641c 100644 --- a/src/postfork.cpp +++ b/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()) { diff --git a/src/postfork.h b/src/postfork.h index 442e666f3..91cc37aa8 100644 --- a/src/postfork.h +++ b/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(); diff --git a/tests/checks/sigint.fish b/tests/checks/sigint.fish index 17916d6c2..bb645fd1d 100644 --- a/tests/checks/sigint.fish +++ b/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 diff --git a/tests/signals.expect b/tests/signals.expect index c7c632c12..360ecfcb3 100644 --- a/tests/signals.expect +++ b/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