github-mirror/fish-shell
github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2025-02-20 17:52:45 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Implement cancel groups

Browse Source 
This concerns how "internal job groups" know to stop executing when an
external command receives a "cancel signal" (SIGINT or SIGQUIT). For
example:

    while true
        sleep 1
    end

The intent is that if any 'sleep' exits from a cancel signal, then so would
the while loop. This is why you can hit control-C to end the loop even
if the SIGINT is delivered to sleep and not fish.

Here the 'while' loop is considered an "internal job group" (no separate
pgid, bash would not fork) while each 'sleep' is a separate external
command with its own job group, pgroup, etc. Prior to this change, after
running each 'sleep', parse_execution_context_t would check to see if its
exit status was a cancel signal, and if so, stash it into an int that the
cancel checker would check. But this became unwieldy: now there were three
sources of cancellation signals (that int, the job group, and fish itself).

Introduce the notion of a "cancellation group" which is a set of job
groups that should cancel together. Even though the while loop and sleep
are in different job groups, they are in the same cancellation group. When
any job gets a SIGINT or SIGQUIT, it marks that signal in its cancellation
group, which prevents running new jobs in that group.

This reduces the number of signals to check from 3 to 2; eventually we can
teach cancellation groups how to check fish's own signals and then it will
just be 1.
This commit is contained in:
ridiculousfish 2020-09-02 15:06:05 -07:00
parent 760b6e76cc
commit 3062994645
7 changed files with 79 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/global_safety.h
View File

@ -97,6 +97,11 @@ class relaxed_atomic_t {
void operator=(T v) { return value_.store(v, std::memory_order_relaxed); }
// Perform a CAS operation, returning whether it succeeded.
bool compare_exchange(T expected, T desired) {
return value_.compare_exchange_strong(expected, desired, std::memory_order_relaxed);
}
// postincrement
T operator++(int) { return value_.fetch_add(1, std::memory_order_relaxed); }

12
src/job_group.cpp
View File

@ -49,8 +49,10 @@ maybe_t<pid_t> job_group_t::get_pgid() const { return pgid_; }
// static
job_group_ref_t job_group_t::resolve_group_for_job(const job_t &job,
const cancellation_group_ref_t &cancel_group,
const job_group_ref_t &proposed) {
assert(!job.group && "Job already has a group");
assert(cancel_group && "Null cancel group");
// Note there's three cases to consider:
// nullptr -> this is a root job, there is no inherited job group
// internal -> the parent is running as part of a simple function execution
@ -77,13 +79,19 @@ job_group_ref_t job_group_t::resolve_group_for_job(const job_t &job,
if (!needs_new_group) return proposed;
// We will need to create a new group.
// We share a cancel group unless we are a background job.
// For example, if we write "begin ; true ; sleep 1 &; end" the `begin` and `true` should cancel
// together, but the `sleep` should not.
cancellation_group_ref_t resolved_cg =
initial_bg ? cancellation_group_t::create() : cancel_group;
properties_t props{};
props.job_control = job.wants_job_control();
props.wants_terminal = job.wants_job_control() && !job.from_event_handler();
props.is_internal = can_use_internal;
props.job_id = can_use_internal ? -1 : acquire_job_id();
job_group_ref_t result{new job_group_t(props, job.command())};
job_group_ref_t result{new job_group_t(props, resolved_cg, job.command())};
// Mark if it's foreground.
result->set_is_foreground(!initial_bg);

53
src/job_group.h
View File

@ -13,6 +13,42 @@
/// 1 is the first valid job ID.
using job_id_t = int;
/// A cancellation group is "a set of jobs that should cancel together." It's effectively just a
/// shared pointer to a bool which latches to true on cancel.
/// For example, in `begin ; true ; end | false`, we have two jobs: the outer pipline and the inner
/// 'true'. These share a cancellation group.
/// Note this is almost but not quite a job group. A job group is a "a set of jobs which share a
/// pgid" but cancellation groups may be bigger. For example in `begin ; sleep 1; sleep 2; end` we
/// have that 'begin' is an internal group (a simple function/block execution) without a pgid,
/// while each 'sleep' will be a different job, with its own pgid, and so be in a different job
/// group. But all share a cancellation group.
/// Note that a background job will always get a new cancellation group.
/// Cancellation groups must be thread safe.
class cancellation_group_t {
public:
/// \return true if we should cancel.
bool should_cancel() const { return get_cancel_signal() != 0; }
/// \return the signal indicating cancellation, or 0 if none.
int get_cancel_signal() const { return signal_; }
/// If we have not already cancelled, then trigger cancellation with the given signal.
void cancel_with_signal(int signal) {
assert(signal > 0 && "Invalid cancel signal");
signal_.compare_exchange(0, signal);
}
/// Helper to return a new group.
static std::shared_ptr<cancellation_group_t> create() {
return std::make_shared<cancellation_group_t>();
}
private:
/// If we cancelled from a signal, return that signal, else 0.
relaxed_atomic_t<int> signal_{0};
};
using cancellation_group_ref_t = std::shared_ptr<cancellation_group_t>;
/// job_group_t is conceptually similar to the idea of a process group. It represents data which
/// is shared among all of the "subjobs" that may be spawned by a single job.
/// For example, two fish functions in a pipeline may themselves spawn multiple jobs, but all will
@ -60,13 +96,13 @@ class job_group_t {
job_id_t get_id() const { return props_.job_id; }
/// Get the cancel signal, or 0 if none.
int get_cancel_signal() const { return cancel_signal_; }
int get_cancel_signal() const { return cancel_group->get_cancel_signal(); }
/// \return the command which produced this job tree.
const wcstring &get_command() const { return command_; }
/// Mark that a process in this group got a signal, and so should cancel.
void set_cancel_signal(int sig) { cancel_signal_ = sig; }
void cancel_with_signal(int sig) { cancel_group->cancel_with_signal(sig); }
/// Mark the root as constructed.
/// This is used to avoid reaping a process group leader while there are still procs that may
@ -78,6 +114,7 @@ class job_group_t {
/// The proposed group is the group from the parent job, or null if this is a root.
/// This never returns null.
static job_group_ref_t resolve_group_for_job(const job_t &job,
const cancellation_group_ref_t &cancel_group,
const job_group_ref_t &proposed_group);
~job_group_t();
@ -87,6 +124,9 @@ class job_group_t {
/// stops.
maybe_t<struct termios> tmodes{};
/// The cancellation group. This is never null.
const cancellation_group_ref_t cancel_group{};
private:
// The pgid to assign to jobs, or none if not yet set.
maybe_t<pid_t> pgid_{};
@ -117,11 +157,10 @@ class job_group_t {
// Whether the root job is constructed. If not, we cannot reap it yet.
relaxed_atomic_bool_t root_constructed_{};
// If not zero, a signal indicating cancellation.
int cancel_signal_{};
job_group_t(const properties_t &props, wcstring command)
: props_(props), command_(std::move(command)) {}
job_group_t(const properties_t &props, cancellation_group_ref_t cg, wcstring command)
: cancel_group(std::move(cg)), props_(props), command_(std::move(command)) {
assert(cancel_group && "Null cancel group");
}
};
#endif

14
src/parse_execution.cpp
View File

@ -119,10 +119,12 @@ static redirection_spec_t get_stderr_merge() {
parse_execution_context_t::parse_execution_context_t(parsed_source_ref_t pstree,
const operation_context_t &ctx,
cancellation_group_ref_t cancel_group,
io_chain_t block_io)
: pstree(std::move(pstree)),
parser(ctx.parser.get()),
ctx(ctx),
cancel_group(std::move(cancel_group)),
block_io(std::move(block_io)) {}
// Utilities
@ -226,7 +228,7 @@ process_type_t parse_execution_context_t::process_type_for_command(
}
maybe_t<end_execution_reason_t> parse_execution_context_t::check_end_execution() const {
if (this->cancel_signal || ctx.check_cancel() || check_cancel_from_fish_signal()) {
if (ctx.check_cancel() || check_cancel_from_fish_signal()) {
return end_execution_reason_t::cancelled;
}
const auto &ld = parser->libdata();
@ -1343,7 +1345,7 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
// Clean up the job on failure or cancellation.
if (pop_result == end_execution_reason_t::ok) {
// Resolve the job's group and mark if this job is the first to get it.
job->group = job_group_t::resolve_group_for_job(*job, ctx.job_group);
job->group = job_group_t::resolve_group_for_job(*job, cancel_group, ctx.job_group);
assert(job->group && "Should not have a null group");
job->mut_flags().is_group_root = (job->group != ctx.job_group);
@ -1364,14 +1366,6 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
remove_job(*this->parser, job.get());
}
// Check if the job's group got a SIGINT or SIGQUIT.
// If so we need to mark that ourselves so as to cancel the rest of the execution.
// See #7259.
int cancel_sig = job->group->get_cancel_signal();
if (cancel_sig == SIGINT || cancel_sig == SIGQUIT) {
this->cancel_signal = cancel_sig;
}
// Update universal variables on external conmmands.
// TODO: justify this, why not on every command?
if (job_contained_external_command) {

13
src/parse_execution.h
View File

@ -12,6 +12,7 @@
#include "proc.h"
class block_t;
class cancellation_group_t;
class operation_context_t;
class parser_t;
@ -37,6 +38,7 @@ class parse_execution_context_t {
parsed_source_ref_t pstree;
parser_t *const parser;
const operation_context_t &ctx;
const std::shared_ptr<cancellation_group_t> cancel_group;
// The currently executing job node, used to indicate the line number.
const ast::job_t *executing_job_node{};
@ -45,10 +47,6 @@ class parse_execution_context_t {
size_t cached_lineno_offset = 0;
int cached_lineno_count = 0;
/// If a process dies due to a SIGINT or SIGQUIT, then store the corresponding signal here.
/// Note this latches to SIGINT or SIGQUIT; it is never cleared.
int cancel_signal{0};
/// The block IO chain.
/// For example, in `begin; foo ; end < file.txt` this would have the 'file.txt' IO.
io_chain_t block_io{};
@ -153,8 +151,10 @@ class parse_execution_context_t {
public:
/// Construct a context in preparation for evaluating a node in a tree, with the given block_io.
/// The execution context may access the parser and group through ctx.
/// The cancel group is never null and should be provided when resolving job groups.
/// The execution context may access the parser and parent job group (if any) through ctx.
parse_execution_context_t(parsed_source_ref_t pstree, const operation_context_t &ctx,
std::shared_ptr<cancellation_group_t> cancel_group,
io_chain_t block_io);
/// Returns the current line number, indexed from 1. Not const since it touches
@ -164,9 +164,6 @@ class parse_execution_context_t {
/// Returns the source offset, or -1.
int get_current_source_offset() const;
/// \return the signal that triggered cancellation, or 0 if none.
int get_cancel_signal() const { return cancel_signal; }
/// Returns the source string.
const wcstring &get_source() const { return pstree->src; }

22
src/parser.cpp
View File

@ -659,6 +659,10 @@ eval_res_t parser_t::eval_node(const parsed_source_ref_t &ps, const T &node,
}
}
// If we are provided a cancellation group, use it; otherwise create one.
cancellation_group_ref_t cancel_group =
job_group ? job_group->cancel_group : cancellation_group_t::create();
// A helper to detect if we got a signal.
// This includes both signals sent to fish (user hit control-C while fish is foreground) and
// signals from the job group (e.g. some external job terminated with SIGQUIT).
@ -666,7 +670,7 @@ eval_res_t parser_t::eval_node(const parsed_source_ref_t &ps, const T &node,
// Did fish itself get a signal?
int sig = signal_check_cancel();
// Has this job group been cancelled?
if (!sig && job_group) sig = job_group->get_cancel_signal();
if (!sig) sig = cancel_group->get_cancel_signal();
return sig;
};
@ -689,8 +693,8 @@ eval_res_t parser_t::eval_node(const parsed_source_ref_t &ps, const T &node,
// Create and set a new execution context.
using exc_ctx_ref_t = std::unique_ptr<parse_execution_context_t>;
scoped_push<exc_ctx_ref_t> exc(&execution_context,
make_unique<parse_execution_context_t>(ps, op_ctx, block_io));
scoped_push<exc_ctx_ref_t> exc(&execution_context, make_unique<parse_execution_context_t>(
ps, op_ctx, cancel_group, block_io));
// Check the exec count so we know if anything got executed.
const size_t prev_exec_count = libdata().exec_count;
@ -699,22 +703,12 @@ eval_res_t parser_t::eval_node(const parsed_source_ref_t &ps, const T &node,
const size_t new_exec_count = libdata().exec_count;
const size_t new_status_count = libdata().status_count;
// Check if the execution context stopped due to a signal from a job it created.
// This may come about if the context created a new job group.
// TODO: there are way too many signals flying around, we need to rationalize this.
int signal_from_exec = execution_context->get_cancel_signal();
exc.restore();
this->pop_block(scope_block);
job_reap(*this, false); // reap again
if (signal_from_exec) {
// A job spawned by the execution context got SIGINT or SIGQUIT, which stopped all
// execution.
return proc_status_t::from_signal(signal_from_exec);
} else if (int sig = check_cancel_signal()) {
// We were signalled.
if (int sig = check_cancel_signal()) {
return proc_status_t::from_signal(sig);
} else {
auto status = proc_status_t::from_exit_code(this->get_last_status());

2
src/proc.cpp
View File

@ -263,7 +263,7 @@ static void handle_child_status(const shared_ptr<job_t> &job, process_t *proc,
if (sig == SIGINT || sig == SIGQUIT) {
if (session_interactivity() != session_interactivity_t::not_interactive) {
// Mark the job group as cancelled.
job->group->set_cancel_signal(sig);
job->group->cancel_with_signal(sig);
} else {
// Deliver the SIGINT or SIGQUIT signal to ourself since we're not interactive.
struct sigaction act;