Migrate the notion of 'foreground' from job to job group

Whether a job is foreground is a property of its pgid, so it belongs
naturally on the job group.
This commit is contained in:
ridiculousfish 2020-07-11 17:01:52 -07:00
parent 0c72e65071
commit 765c48afa4
6 changed files with 64 additions and 40 deletions

View file

@ -30,7 +30,7 @@ static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) {
streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id(),
j->command_wcstr());
parser.job_promote(j);
j->mut_flags().foreground = false;
j->group->set_is_foreground(false);
j->continue_job(parser, true, j->is_stopped());
return STATUS_CMD_OK;
}

View file

@ -104,7 +104,7 @@ int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
reader_write_title(job->command(), parser);
parser.job_promote(job);
job->mut_flags().foreground = true;
job->group->set_is_foreground(true);
job->continue_job(parser, true, job->is_stopped());
return STATUS_CMD_OK;

View file

@ -136,7 +136,7 @@ static bool can_use_posix_spawn_for_job(const std::shared_ptr<job_t> &job,
if (job->wants_job_control()) { //!OCLINT(collapsible if statements)
// We are going to use job control; therefore when we launch this job it will get its own
// process group ID. But will it be foregrounded?
if (job->should_claim_terminal()) {
if (job->group->should_claim_terminal()) {
// It will be foregrounded, so we will call tcsetpgrp(), therefore do not use
// posix_spawn.
return false;
@ -321,7 +321,8 @@ static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t
if (int err = execute_setpgid(p->pid, pgid, false /* not parent */)) {
report_setpgid_error(err, pgid, job.get(), p);
}
child_setup_process(job->should_claim_terminal() ? pgid : INVALID_PID, *job, true, dup2s);
child_setup_process(job->group->should_claim_terminal() ? pgid : INVALID_PID, *job, true,
dup2s);
child_action();
DIE("Child process returned control to fork_child lambda!");
}

View file

@ -992,7 +992,7 @@ end_execution_reason_t parse_execution_context_t::populate_not_process(
flags.negate = !flags.negate;
if (not_statement.time) {
flags.has_time_prefix = true;
if (!job->mut_flags().foreground) {
if (job->is_initially_background()) {
return this->report_error(STATUS_INVALID_ARGS, not_statement, ERROR_TIME_BACKGROUND);
}
}
@ -1313,8 +1313,6 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
shared_ptr<job_t> job = std::make_shared<job_t>(props);
job->tmodes = tmodes;
job->mut_flags().foreground = !props.initial_background;
// We are about to populate a job. One possible argument to the job is a command substitution
// which may be interested in the job that's populating it, via '--on-job-exit caller'. Record
// the job ID here.

View file

@ -241,14 +241,9 @@ void print_exit_warning_for_jobs(const job_list_t &jobs) {
fputws(_(L"Use 'disown PID' to remove jobs from the list without terminating them.\n"), stdout);
}
job_group_t::job_group_t(bool job_control, bool internal)
: job_control_(job_control),
is_internal_(internal),
job_id_(internal ? -1 : acquire_job_id()) {}
job_group_t::~job_group_t() {
if (job_id_ > 0) {
release_job_id(job_id_);
if (props_.job_id > 0) {
release_job_id(props_.job_id);
}
}
@ -270,15 +265,16 @@ void job_group_t::populate_tree_for_job(job_t *job, const job_group_ref_t &propo
// non-internal -> we are running as part of a real pipeline
// Decide if this job can use an internal tree.
// This is true if it's a simple foreground execution of an internal proc.
bool initial_bg = job->is_initially_background();
bool first_proc_internal = job->processes.front()->is_internal();
bool can_use_internal = !job->is_initially_background() && job->processes.size() == 1 &&
job->processes.front()->is_internal();
bool can_use_internal =
!initial_bg && job->processes.size() == 1 && job->processes.front()->is_internal();
bool needs_new_tree = false;
if (!proposed) {
// We don't have a tree yet.
needs_new_tree = true;
} else if (!job->is_foreground()) {
} else if (initial_bg) {
// Background jobs always get a new tree.
needs_new_tree = true;
} else if (proposed->is_internal() && !can_use_internal) {
@ -287,20 +283,25 @@ void job_group_t::populate_tree_for_job(job_t *job, const job_group_ref_t &propo
}
job->mut_flags().is_tree_root = needs_new_tree;
bool job_control = job->wants_job_control();
if (!needs_new_tree) {
job->group = proposed;
} else if (can_use_internal) {
job->group.reset(new job_group_t(job_control, true));
} else {
job->group.reset(new job_group_t(job_control, false));
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.reset(new job_group_t(props));
// Mark if it's foreground.
job->group->set_is_foreground(!initial_bg);
// Perhaps this job should immediately live in fish's pgroup.
// There's two reasons why it may be so:
// 1. The job doesn't need job control.
// 2. The first process in the job is internal to fish; this needs to own the tty.
if (!job_control || first_proc_internal) {
if (!can_use_internal && (!props.job_control || first_proc_internal)) {
job->group->set_pgid(getpgrp());
}
}
@ -497,7 +498,7 @@ static void process_mark_finished_children(parser_t &parser, bool block_ok) {
assert(pid == proc->pid && "Unexpcted waitpid() return");
handle_child_status(proc.get(), proc_status_t::from_waitpid(status));
if (proc->status.stopped()) {
j->mut_flags().foreground = false;
j->group->set_is_foreground(false);
}
if (proc->status.continued()) {
j->mut_flags().notified = false;
@ -802,7 +803,7 @@ void proc_update_jiffies(parser_t &parser) {
int terminal_maybe_give_to_job(const job_t *j, bool continuing_from_stopped) {
enum { notneeded = 0, success = 1, error = -1 };
if (!j->should_claim_terminal()) {
if (!j->group->should_claim_terminal()) {
// The job doesn't want the terminal.
return notneeded;
}

View file

@ -188,17 +188,26 @@ class job_group_t {
maybe_t<pid_t> get_pgid() const;
/// \return whether we want job control
bool wants_job_control() const { return job_control_; }
bool wants_job_control() const { return props_.job_control; }
/// \return whether this is an internal group.
bool is_internal() const { return is_internal_; }
bool is_internal() const { return props_.is_internal; }
/// \return whether we are currently the foreground group.
bool is_foreground() const { return is_foreground_; }
/// Mark whether we are in the foreground.
void set_is_foreground(bool flag) { is_foreground_ = flag; }
/// \return if this job group should own the terminal when it runs.
bool should_claim_terminal() const { return props_.wants_terminal && is_foreground(); }
/// \return whether this job group is awaiting a pgid.
/// This is true for non-internal trees that don't already have a pgid.
bool needs_pgid_assignment() const { return !is_internal_ && !pgid_.has_value(); }
bool needs_pgid_assignment() const { return !props_.is_internal && !pgid_.has_value(); }
/// \return the job ID, or -1 if none.
job_id_t get_id() const { return job_id_; }
job_id_t get_id() const { return props_.job_id; }
/// Mark the root as constructed.
/// This is used to avoid reaping a process group leader while there are still procs that may
@ -213,13 +222,33 @@ class job_group_t {
~job_group_t();
private:
// The pgid to assign to jobs, or none if not yet set.
maybe_t<pid_t> pgid_{};
const bool job_control_;
const bool is_internal_;
const job_id_t job_id_;
// Set of properties, which are constant.
struct properties_t {
// Whether jobs in this group should have job control.
bool job_control{};
// Whether we should claim the terminal when we run in the foreground.
// TODO: this is effectively the same as job control, rationalize this.
bool wants_terminal{};
// Whether we are an internal job group.
bool is_internal{};
// The job ID of this group.
job_id_t job_id{};
};
const properties_t props_;
// Whether we are in the foreground, meaning that the user is waiting for this.
relaxed_atomic_bool_t is_foreground_{};
// Whether the root job is constructed. If not, we cannot reap it yet.
relaxed_atomic_bool_t root_constructed_{};
explicit job_group_t(bool job_control, bool internal);
explicit job_group_t(const properties_t &props) : props_(props) {}
};
/// A structure representing a single fish process. Contains variables for tracking process state
@ -474,9 +503,6 @@ class job_t {
/// Whether the user has been told about stopped job.
bool notified{false};
/// Whether this job is in the foreground.
bool foreground{false};
/// Whether the exit status should be negated. This flag can only be set by the not builtin.
bool negate{false};
@ -500,9 +526,6 @@ class job_t {
/// \return if we want job control.
bool wants_job_control() const { return properties.job_control; }
/// \return if this job should own the terminal when it runs.
bool should_claim_terminal() const { return properties.wants_terminal && is_foreground(); }
/// \return whether this job is initially going to run in the background, because & was
/// specified.
bool is_initially_background() const { return properties.initial_background; }
@ -519,8 +542,6 @@ class job_t {
// Helper functions to check presence of flags on instances of jobs
/// The job has been fully constructed, i.e. all its member processes have been launched
bool is_constructed() const { return flags().constructed; }
/// The job was launched in the foreground and has control of the terminal
bool is_foreground() const { return flags().foreground; }
/// The job is complete, i.e. all its member processes have been reaped
bool is_completed() const;
/// The job is in a stopped state
@ -532,6 +553,9 @@ class job_t {
bool skip_notification() const { return properties.skip_notification; }
bool from_event_handler() const { return properties.from_event_handler; }
/// \return whether this job's group is in the foreground.
bool is_foreground() const { return group->is_foreground(); }
/// \return whether we should report process exit events.
/// This implements some historical behavior which has not been justified.
bool should_report_process_exits() const;