h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-06-03 06:41:14 -03:00
Code Issues Packages Projects Releases Wiki Activity

Refactor job pgroup assignment

Browse Source 
This is a cleanup of job groups, rationalizing a bunch of stuff. Some
notable changes (none user-visible hopefully):

1. Previously, if a job group wanted a pgid, then we would assign it to the
   first process to run in the job group. Now we deliberately mark which
   process will own the pgroup, via a new `leads_pgrp` flag in process_t. This
   eliminates a source of ambiguity.

2. Previously, if a job were run inside fish's pgroup, we would set fish's
   pgroup as the group of the job. But this meant we had to check if the job
   had fish's pgroup in lots of places, for example when calling tcsetpgrp.
   Now a job group only has a pgrp if that pgrp is external (i.e. the job is
   under job control).
This commit is contained in:
ridiculousfish
2022-02-19 10:05:50 -08:00
parent 5994e44877
commit 3f585cddfc
8 changed files with 194 additions and 284 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
src/exec.cpp
View File

@@ -222,14 +222,10 @@ static bool can_use_posix_spawn_for_job(const std::shared_ptr<job_t> &job,
for (const auto &action : dup2s.get_actions()) { for (const auto &action : dup2s.get_actions()) {
if (action.src == action.target) return false; if (action.src == action.target) return false;
} }
if (job->wants_job_control()) { //!OCLINT(collapsible if statements) if (job->group->wants_terminal()) {
// We are going to use job control; therefore when we launch this job it will get its own // This job will be foregrounded, so we will call tcsetpgrp(), therefore do not use
// process group ID. But will it be foregrounded? // posix_spawn.
if (job->group->should_claim_terminal()) { return false;
// It will be foregrounded, so we will call tcsetpgrp(), therefore do not use
// posix_spawn.
return false;
}
} }
return true; return true;
} }
@@ -263,16 +259,6 @@ static void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &block_i
} }
} }
/// If our pgroup assignment mode wants us to use the first external proc, then apply it here.
/// \returns the job's pgid, which should always be set to something valid after this call.
static pid_t maybe_assign_pgid_from_child(const std::shared_ptr<job_t> &j, pid_t child_pid) {
auto &jt = j->group;
if (jt->needs_pgid_assignment()) {
jt->set_pgid(child_pid);
}
return *jt->get_pgid();
}
/// Construct an internal process for the process p. In the background, write the data \p outdata to /// Construct an internal process for the process p. In the background, write the data \p outdata to
/// stdout and \p errdata to stderr, respecting the io chain \p ios. For example if target_fd is 1 /// stdout and \p errdata to stderr, respecting the io chain \p ios. For example if target_fd is 1
/// (stdout), and there is a dup2 3->1, then we need to write to fd 3. Then exit the internal /// (stdout), and there is a dup2 3->1, then we need to write to fd 3. Then exit the internal
@@ -411,46 +397,40 @@ bool blocked_signals_for_job(const job_t &job, sigset_t *sigmask) {
static launch_result_t fork_child_for_process(const std::shared_ptr<job_t> &job, process_t *p, static launch_result_t fork_child_for_process(const std::shared_ptr<job_t> &job, process_t *p,
const dup2_list_t &dup2s, const char *fork_type, const dup2_list_t &dup2s, const char *fork_type,
const std::function<void()> &child_action) { const std::function<void()> &child_action) {
assert(!job->group->is_internal() && "Internal groups should never need to fork");
// Decide if we want to job to control the tty. // Decide if we want to job to control the tty.
// If so we need to get our pgroup; if not we don't need the pgroup. // If so we need to get our pgroup; if not we don't need the pgroup.
bool claim_tty = job->group->should_claim_terminal(); bool claim_tty = job->group->wants_terminal();
pid_t fish_pgrp = claim_tty ? getpgrp() : INVALID_PID; pid_t fish_pgrp = claim_tty ? getpgrp() : INVALID_PID;
pid_t pid = execute_fork(); pid_t pid = execute_fork();
if (pid == 0) { if (pid < 0) {
// This is the child process. Setup redirections, print correct output to return launch_result_t::failed;
// stdout and stderr, and then exit. }
p->pid = getpid(); const bool is_parent = (pid > 0);
pid_t pgid = maybe_assign_pgid_from_child(job, p->pid);
// The child attempts to join the pgroup. // Record the pgroup if this is the leader.
if (int err = execute_setpgid(p->pid, pgid, false /* not parent */)) { // Both parent and child attempt to send the process to its new group, to resolve the race.
report_setpgid_error(err, false /* is_parent */, pgid, job.get(), p); p->pid = is_parent ? pid : getpid();
if (p->leads_pgrp) {
job->group->set_pgid(p->pid);
}
if (auto pgid = job->group->get_pgid()) {
if (int err = execute_setpgid(p->pid, *pgid, is_parent)) {
report_setpgid_error(err, is_parent, *pgid, job.get(), p);
} }
child_setup_process(claim_tty ? pgid : INVALID_PID, fish_pgrp, *job, true, dup2s); }
if (!is_parent) {
// Child process.
child_setup_process(claim_tty ? *job->group->get_pgid() : INVALID_PID, fish_pgrp, *job,
true, dup2s);
child_action(); child_action();
DIE("Child process returned control to fork_child lambda!"); DIE("Child process returned control to fork_child lambda!");
} }
if (pid < 0) {
return launch_result_t::failed;
}
// This is the parent process. Store away information on the child, and
// possibly give it control over the terminal.
s_fork_count++; s_fork_count++;
FLOGF(exec_fork, L"Fork #%d, pid %d: %s for '%ls'", int(s_fork_count), pid, fork_type, FLOGF(exec_fork, L"Fork #%d, pid %d: %s for '%ls'", int(s_fork_count), pid, fork_type,
p->argv0()); p->argv0());
p->pid = pid;
pid_t pgid = maybe_assign_pgid_from_child(job, p->pid);
// The parent attempts to send the child to its pgroup.
// EACCESS is an expected benign error as the child may have called exec().
if (int err = execute_setpgid(p->pid, pgid, true /* is parent */)) {
if (err != EACCES) report_setpgid_error(err, true /* is_parent */, pgid, job.get(), p);
}
terminal_maybe_give_to_job_group(job->group.get(), false); terminal_maybe_give_to_job_group(job->group.get(), false);
return launch_result_t::ok; return launch_result_t::ok;
} }
@@ -573,13 +553,14 @@ static launch_result_t exec_external_command(parser_t &parser, const std::shared
// these are all things do_fork() takes care of normally (for forked processes): // these are all things do_fork() takes care of normally (for forked processes):
p->pid = *pid; p->pid = *pid;
pid_t pgid = maybe_assign_pgid_from_child(j, p->pid); if (p->leads_pgrp) {
j->group->set_pgid(p->pid);
// posix_spawn should in principle set the pgid before returning. // posix_spawn should in principle set the pgid before returning.
// In glibc, posix_spawn uses fork() and the pgid group is set on the child side; // In glibc, posix_spawn uses fork() and the pgid group is set on the child side;
// therefore the parent may not have seen it be set yet. // therefore the parent may not have seen it be set yet.
// Ensure it gets set. See #4715, also https://github.com/Microsoft/WSL/issues/2997. // Ensure it gets set. See #4715, also https://github.com/Microsoft/WSL/issues/2997.
execute_setpgid(p->pid, pgid, true /* is parent */); execute_setpgid(p->pid, p->pid, true /* is parent */);
}
terminal_maybe_give_to_job_group(j->group.get(), false); terminal_maybe_give_to_job_group(j->group.get(), false);
return launch_result_t::ok; return launch_result_t::ok;
} else } else
@@ -1124,8 +1105,7 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const io_chain_t &bl
} }
} }
FLOGF(exec_job_exec, L"Executed job %d from command '%ls' with pgrp %d", j->job_id(), FLOGF(exec_job_exec, L"Executed job %d from command '%ls'", j->job_id(), j->command_wcstr());
j->command_wcstr(), j->get_pgid() ? *j->get_pgid() : -2);
j->mark_constructed(); j->mark_constructed();

95
src/job_group.cpp
View File

@@ -31,77 +31,40 @@ static void release_job_id(job_id_t jid) {
consumed_job_ids->erase(where); consumed_job_ids->erase(where);
} }
job_group_t::job_group_t(wcstring command, cancellation_group_ref_t cg, job_id_t job_id,
bool job_control, bool wants_terminal)
: cancel_group(std::move(cg)),
job_control_(job_control),
wants_terminal_(wants_terminal),
command_(std::move(command)),
job_id_(job_id) {}
job_group_t::~job_group_t() { job_group_t::~job_group_t() {
if (props_.job_id > 0) { if (job_id_ > 0) {
release_job_id(props_.job_id); release_job_id(job_id_);
} }
} }
// static
job_group_ref_t job_group_t::create(wcstring command, cancellation_group_ref_t cg,
bool wants_job_id) {
job_id_t jid = wants_job_id ? acquire_job_id() : 0;
return job_group_ref_t(new job_group_t(std::move(command), std::move(cg), jid));
}
// static
job_group_ref_t job_group_t::create_with_job_control(wcstring command, cancellation_group_ref_t cg,
bool wants_terminal) {
return job_group_ref_t(new job_group_t(std::move(command), std::move(cg), acquire_job_id(),
true /* job_control */, wants_terminal));
}
void job_group_t::set_pgid(pid_t pgid) { void job_group_t::set_pgid(pid_t pgid) {
// Note we need not be concerned about thread safety. job_groups are intended to be shared // Note we need not be concerned about thread safety. job_groups are intended to be shared
// across threads, but their pgid should always have been set beforehand. // across threads, but any pgid should always have been set beforehand, since it's set
assert(needs_pgid_assignment() && "We should not be setting a pgid"); // immediately after thfe first process launches.
assert(pgid >= 0 && "Invalid pgid"); assert(pgid >= 0 && "invalid pgid");
assert(wants_job_control() && "should not set a pgid for this group");
assert(!pgid_.has_value() && "pgid already set");
pgid_ = pgid; pgid_ = pgid;
} }
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
// We may need to create a new job group if we are going to fork.
// non-internal -> we are running as part of a real pipeline
// Decide if this job can use an internal group.
// 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 =
!initial_bg && job.processes.size() == 1 && job.processes.front()->is_internal();
bool needs_new_group = false;
if (!proposed) {
// We don't have a group yet.
needs_new_group = true;
} else if (initial_bg) {
// Background jobs always get a new group.
needs_new_group = true;
} else if (proposed->is_internal() && !can_use_internal) {
// We cannot use the internal group for this job.
needs_new_group = true;
}
if (!needs_new_group) return proposed;
// 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, resolved_cg, job.command())};
// Mark if it's foreground.
result->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 (!can_use_internal && (!props.job_control || first_proc_internal)) {
result->set_pgid(getpgrp());
}
return result;
}

117
src/job_group.h
View File

@@ -64,20 +64,11 @@ using job_group_ref_t = std::shared_ptr<job_group_t>;
class job_group_t { class job_group_t {
public: public:
/// Set the pgid for this job group, latching it to this value. /// \return whether this group wants job control.
/// The pgid should not already have been set. bool wants_job_control() const { return job_control_; }
/// Of course this does not keep the pgid alive by itself.
/// An internal job group does not have a pgid and it is an error to set it.
void set_pgid(pid_t pgid);
/// Get the pgid, or none() if it has not been set. /// \return if this job group should own the terminal when it runs.
maybe_t<pid_t> get_pgid() const; bool wants_terminal() const { return wants_terminal_ && is_foreground(); }
/// \return whether we want job control
bool wants_job_control() const { return props_.job_control; }
/// \return whether this is an internal group.
bool is_internal() const { return props_.is_internal; }
/// \return whether we are currently the foreground group. /// \return whether we are currently the foreground group.
bool is_foreground() const { return is_foreground_; } bool is_foreground() const { return is_foreground_; }
@@ -85,82 +76,70 @@ class job_group_t {
/// Mark whether we are in the foreground. /// Mark whether we are in the foreground.
void set_is_foreground(bool flag) { is_foreground_ = flag; } void set_is_foreground(bool flag) { is_foreground_ = flag; }
/// \return if this job group should own the terminal when it runs. /// \return the command which produced this job tree.
bool should_claim_terminal() const { return props_.wants_terminal && is_foreground(); } const wcstring &get_command() const { return command_; }
/// \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 !props_.is_internal && !pgid_.has_value(); }
/// \return the job ID, or -1 if none. /// \return the job ID, or -1 if none.
job_id_t get_id() const { return props_.job_id; } job_id_t get_job_id() const { return job_id_; }
/// \return whether we have a valid job ID. "Simple block" groups like function calls do not.
bool has_job_id() const { return job_id_ > 0; }
/// Get the cancel signal, or 0 if none. /// Get the cancel signal, or 0 if none.
int get_cancel_signal() const { return cancel_group->get_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. /// Mark that a process in this group got a signal, and so should cancel.
void cancel_with_signal(int sig) { cancel_group->cancel_with_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
/// want to enter its group.
void mark_root_constructed() { root_constructed_ = true; };
bool is_root_constructed() const { return root_constructed_; }
/// Given a job and a proposed job group (possibly null), return a group for the job.
/// 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();
/// If set, the saved terminal modes of this job. This needs to be saved so that we can restore
/// the terminal to the same state after temporarily taking control over the terminal when a job
/// stops.
maybe_t<struct termios> tmodes{};
/// The cancellation group. This is never null. /// The cancellation group. This is never null.
const cancellation_group_ref_t cancel_group{}; const cancellation_group_ref_t cancel_group{};
/// If set, the saved terminal modes of this job. This needs to be saved so that we can restore
/// the terminal to the same state when resuming a stopped job.
maybe_t<struct termios> tmodes{};
/// Set the pgid for this job group, latching it to this value.
/// This should only be called if job control is active for this group.
/// The pgid should not already have been set, and should be different from fish's pgid.
/// Of course this does not keep the pgid alive by itself.
void set_pgid(pid_t pgid);
/// Get the pgid. This never returns fish's pgid.
maybe_t<pid_t> get_pgid() const { return pgid_; }
/// Construct a group for a job that will live internal to fish, optionally claiming a job ID.
static job_group_ref_t create(wcstring command, cancellation_group_ref_t cg, bool wants_job_id);
/// Construct a group for a job which will assign its first process as pgroup leader.
static job_group_ref_t create_with_job_control(wcstring command, cancellation_group_ref_t cg,
bool wants_terminal);
~job_group_t();
private: private:
// The pgid to assign to jobs, or none if not yet set. job_group_t(wcstring command, cancellation_group_ref_t cg, job_id_t job_id,
maybe_t<pid_t> pgid_{}; bool job_control = false, bool wants_terminal = false);
// Set of properties, which are constant. // Whether job control is enabled.
struct properties_t { // If this is set, then the first process in the root job must be external.
// Whether jobs in this group should have job control. // It will become the process group leader.
bool job_control{}; const bool job_control_;
// Whether we should claim the terminal when we run in the foreground. // Whether we should tcsetpgrp to the job when it runs in the foreground.
// TODO: this is effectively the same as job control, rationalize this. const bool wants_terminal_;
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_;
// The original command which produced this job tree.
const wcstring command_;
// Whether we are in the foreground, meaning that the user is waiting for this. // Whether we are in the foreground, meaning that the user is waiting for this.
relaxed_atomic_bool_t is_foreground_{}; relaxed_atomic_bool_t is_foreground_{};
// Whether the root job is constructed. If not, we cannot reap it yet. // The pgid leading our group. This is only ever set if job_control_ is true.
relaxed_atomic_bool_t root_constructed_{}; // This is never fish's pgid.
maybe_t<pid_t> pgid_{};
job_group_t(const properties_t &props, cancellation_group_ref_t cg, wcstring command) // The original command which produced this job tree.
: cancel_group(std::move(cg)), props_(props), command_(std::move(command)) { const wcstring command_;
assert(cancel_group && "Null cancel group");
} /// Our job ID. -1 if none.
const job_id_t job_id_;
}; };
#endif #endif

62
src/parse_execution.cpp
View File

@@ -1338,20 +1338,11 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
const auto &ld = parser->libdata(); const auto &ld = parser->libdata();
auto job_control_mode = get_job_control_mode();
// Run all command substitutions in our pgroup.
bool wants_job_control =
!parser->is_command_substitution() &&
((job_control_mode == job_control_t::all) ||
((job_control_mode == job_control_t::interactive) && parser->is_interactive()) ||
(ctx.job_group && ctx.job_group->wants_job_control()));
job_t::properties_t props{}; job_t::properties_t props{};
props.initial_background = job_node.bg.has_value(); props.initial_background = job_node.bg.has_value();
props.skip_notification = props.skip_notification =
ld.is_subshell || parser->is_block() || ld.is_event || !parser->is_interactive(); ld.is_subshell || parser->is_block() || ld.is_event || !parser->is_interactive();
props.from_event_handler = ld.is_event; props.from_event_handler = ld.is_event;
props.job_control = wants_job_control;
props.wants_timing = job_node_wants_timing(job_node); props.wants_timing = job_node_wants_timing(job_node);
// It's an error to have 'time' in a background job. // It's an error to have 'time' in a background job.
@@ -1374,23 +1365,12 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
// Clean up the job on failure or cancellation. // Clean up the job on failure or cancellation.
if (pop_result == end_execution_reason_t::ok) { if (pop_result == end_execution_reason_t::ok) {
// Resolve the job's group and mark if this job is the first to get it. this->setup_group(job.get());
job->group = job_group_t::resolve_group_for_job(*job, cancel_group, ctx.job_group);
assert(job->group && "Should not have a null group"); assert(job->group && "Should not have a null group");
job->mut_flags().is_group_root = (job->group != ctx.job_group);
// Success. Give the job to the parser - it will clean it up. // Give the job to the parser - it will clean it up.
parser->job_add(job); parser->job_add(job);
// Check to see if this contained any external commands.
bool job_contained_external_command = false;
for (const auto &proc : job->processes) {
if (proc->type == process_type_t::external) {
job_contained_external_command = true;
break;
}
}
// Actually execute the job. // Actually execute the job.
if (!exec_job(*parser, job, block_io)) { if (!exec_job(*parser, job, block_io)) {
// No process in the job successfully launched. // No process in the job successfully launched.
@@ -1404,7 +1384,7 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
// Update universal variables on external commands. // Update universal variables on external commands.
// TODO: justify this, why not on every command? // TODO: justify this, why not on every command?
if (job_contained_external_command) { if (job->has_external_proc()) {
parser->vars().universal_barrier(); parser->vars().universal_barrier();
} }
} }
@@ -1545,6 +1525,42 @@ end_execution_reason_t parse_execution_context_t::eval_node(const ast::job_list_
return this->run_job_list(job_list, associated_block); return this->run_job_list(job_list, associated_block);
} }
void parse_execution_context_t::setup_group(job_t *j) {
// We can use the parent group if it's compatible and we're not backgrounded.
if (ctx.job_group && (ctx.job_group->has_job_id() || !j->wants_job_id()) &&
!j->is_initially_background()) {
j->group = ctx.job_group;
return;
}
if (j->processes.front()->is_internal() || !this->use_job_control()) {
// This job either doesn't have a pgroup (e.g. a simple block), or lives in fish's pgroup.
j->group = job_group_t::create(j->command(), cancel_group, j->wants_job_id());
} else {
// This is a "real job" that gets its own pgroup.
j->processes.front()->leads_pgrp = true;
bool wants_terminal = !parser->libdata().is_event;
j->group = job_group_t::create_with_job_control(j->command(), cancel_group, wants_terminal);
}
j->group->set_is_foreground(!j->is_initially_background());
j->mut_flags().is_group_root = true;
}
bool parse_execution_context_t::use_job_control() const {
if (parser->is_command_substitution()) {
return false;
}
switch (get_job_control_mode()) {
case job_control_t::all:
return true;
case job_control_t::interactive:
return parser->is_interactive();
case job_control_t::none:
return false;
}
DIE("Unreachable");
}
int parse_execution_context_t::line_offset_of_node(const ast::job_t *node) { int parse_execution_context_t::line_offset_of_node(const ast::job_t *node) {
// If we're not executing anything, return -1. // If we're not executing anything, return -1.
if (!node) { if (!node) {

6
src/parse_execution.h
View File

@@ -141,6 +141,12 @@ class parse_execution_context_t : noncopyable_t {
end_execution_reason_t populate_job_from_job_node(job_t *j, const ast::job_t &job_node, end_execution_reason_t populate_job_from_job_node(job_t *j, const ast::job_t &job_node,
const block_t *associated_block); const block_t *associated_block);
// Assign a job group to the given job.
void setup_group(job_t *j);
// \return whether we should apply job control to our processes.
bool use_job_control() const;
// Returns the line number of the node. Not const since it touches cached_lineno_offset. // Returns the line number of the node. Not const since it touches cached_lineno_offset.
int line_offset_of_node(const ast::job_t *node); int line_offset_of_node(const ast::job_t *node);
int line_offset_of_character_at_offset(size_t offset); int line_offset_of_character_at_offset(size_t offset);

9
src/postfork.cpp
View File

@@ -283,12 +283,9 @@ posix_spawner_t::posix_spawner_t(const job_t *j, const dup2_list_t &dup2s) {
// desired_pgid tracks the pgroup for the process. If it is none, the pgroup is left unchanged. // desired_pgid tracks the pgroup for the process. If it is none, the pgroup is left unchanged.
// If it is zero, create a new pgroup from the pid. If it is >0, join that pgroup. // If it is zero, create a new pgroup from the pid. If it is >0, join that pgroup.
maybe_t<pid_t> desired_pgid = none(); maybe_t<pid_t> desired_pgid = none();
if (auto job_pgid = j->group->get_pgid()) { if (auto pgid = j->group->get_pgid()) {
desired_pgid = *job_pgid; desired_pgid = *pgid;
} else { } else if (j->processes.front()->leads_pgrp) {
assert(j->group->needs_pgid_assignment() && "We should be expecting a pgid");
// We are the first external proc in the job group. Set the desired_pgid to 0 to indicate we
// should creating a new process group.
desired_pgid = 0; desired_pgid = 0;
} }

85
src/proc.cpp
View File

@@ -120,9 +120,6 @@ bool job_t::is_completed() const {
} }
bool job_t::posts_job_exit_events() const { bool job_t::posts_job_exit_events() const {
// If we never got a pgid then we never launched the external process, so don't report it.
if (!this->get_pgid()) return false;
// Only report root job exits. // Only report root job exits.
// For example in `ls | begin ; cat ; end` we don't need to report the cat sub-job. // For example in `ls | begin ; cat ; end` we don't need to report the cat sub-job.
if (!flags().is_group_root) return false; if (!flags().is_group_root) return false;
@@ -131,14 +128,8 @@ bool job_t::posts_job_exit_events() const {
return this->has_external_proc(); return this->has_external_proc();
} }
bool job_t::job_chain_is_fully_constructed() const { return group->is_root_constructed(); }
bool job_t::signal(int signal) { bool job_t::signal(int signal) {
// Presumably we are distinguishing between the two cases below because we do if (auto pgid = group->get_pgid()) {
// not want to send ourselves the signal in question in case the job shares
// a pgid with the shell.
auto pgid = get_pgid();
if (pgid.has_value() && *pgid != getpgrp()) {
if (killpg(*pgid, signal) == -1) { if (killpg(*pgid, signal) == -1) {
char buffer[512]; char buffer[512];
sprintf(buffer, "killpg(%d, %s)", *pgid, strsignal(signal)); sprintf(buffer, "killpg(%d, %s)", *pgid, strsignal(signal));
@@ -152,7 +143,6 @@ bool job_t::signal(int signal) {
} }
} }
} }
return true; return true;
} }
@@ -312,12 +302,11 @@ job_t::job_t(const properties_t &props, wcstring command_str)
job_t::~job_t() = default; job_t::~job_t() = default;
bool job_t::wants_job_control() const { return group->wants_job_control(); }
void job_t::mark_constructed() { void job_t::mark_constructed() {
assert(!is_constructed() && "Job was already constructed"); assert(!is_constructed() && "Job was already constructed");
mut_flags().constructed = true; mut_flags().constructed = true;
if (flags().is_group_root) {
group->mark_root_constructed();
}
} }
bool job_t::has_internal_proc() const { bool job_t::has_internal_proc() const {
@@ -334,27 +323,20 @@ bool job_t::has_external_proc() const {
return false; return false;
} }
/// A list of pids/pgids that have been disowned. They are kept around until either they exit or bool job_t::wants_job_id() const {
return processes.size() > 1 || !processes.front()->is_internal() || is_initially_background();
}
/// A list of pids that have been disowned. They are kept around until either they exit or
/// we exit. Poll these from time-to-time to prevent zombie processes from happening (#5342). /// we exit. Poll these from time-to-time to prevent zombie processes from happening (#5342).
static owning_lock<std::vector<pid_t>> s_disowned_pids; static owning_lock<std::vector<pid_t>> s_disowned_pids;
void add_disowned_job(const job_t *j) { void add_disowned_job(const job_t *j) {
assert(j && "Null job"); assert(j && "Null job");
// Never add our own (or an invalid) pgid as it is not unique to only
// one job, and may result in a deadlock if we attempt the wait.
auto pgid = j->get_pgid();
auto disowned_pids = s_disowned_pids.acquire(); auto disowned_pids = s_disowned_pids.acquire();
if (pgid && *pgid != getpgrp() && *pgid > 0) { for (auto &process : j->processes) {
// waitpid(2) is signalled to wait on a process group rather than a if (process->pid) {
// process id by using the negative of its value. disowned_pids->push_back(process->pid);
disowned_pids->push_back(*pgid * -1);
} else {
// Instead, add the PIDs of any external processes
for (auto &process : j->processes) {
if (process->pid) {
disowned_pids->push_back(process->pid);
}
} }
} }
} }
@@ -630,7 +612,7 @@ static void remove_disowned_jobs(job_list_t &jobs) {
auto iter = jobs.begin(); auto iter = jobs.begin();
while (iter != jobs.end()) { while (iter != jobs.end()) {
const auto &j = *iter; const auto &j = *iter;
if (j->flags().disown_requested && j->job_chain_is_fully_constructed()) { if (j->flags().disown_requested && j->is_constructed()) {
iter = jobs.erase(iter); iter = jobs.erase(iter);
} else { } else {
++iter; ++iter;
@@ -810,20 +792,14 @@ void proc_update_jiffies(parser_t &parser) {
// restoring a previously-stopped job, in which case we need to restore terminal attributes. // restoring a previously-stopped job, in which case we need to restore terminal attributes.
int terminal_maybe_give_to_job_group(const job_group_t *jg, bool continuing_from_stopped) { int terminal_maybe_give_to_job_group(const job_group_t *jg, bool continuing_from_stopped) {
enum { notneeded = 0, success = 1, error = -1 }; enum { notneeded = 0, success = 1, error = -1 };
if (!jg->wants_terminal() || !jg->get_pgid()) {
if (!jg->should_claim_terminal()) { // The job doesn't want the terminal, or doesn't have a pgroup yet.
// The job doesn't want the terminal.
return notneeded; return notneeded;
} }
// Get the pgid; we may not have one. // Get the pgid.
pid_t pgid{}; pid_t pgid = *jg->get_pgid();
if (auto mpgid = jg->get_pgid()) { assert(pgid >= 0 && "Invalid pgid");
pgid = *mpgid;
} else {
FLOG(proc_termowner, L"terminal_give_to_job() returning early due to no process group");
return notneeded;
}
// If we are continuing, ensure that stdin is marked as blocking first (issue #176). // If we are continuing, ensure that stdin is marked as blocking first (issue #176).
// Also restore tty modes. // Also restore tty modes.
@@ -933,7 +909,7 @@ int terminal_maybe_give_to_job_group(const job_group_t *jg, bool continuing_from
return notneeded; return notneeded;
} else { } else {
FLOGF(warning, _(L"Could not send job %d ('%ls') with pgid %d to foreground"), FLOGF(warning, _(L"Could not send job %d ('%ls') with pgid %d to foreground"),
jg->get_id(), jg->get_command().c_str(), pgid); jg->get_job_id(), jg->get_command().c_str(), pgid);
wperror(L"tcsetpgrp"); wperror(L"tcsetpgrp");
return error; return error;
} }
@@ -956,33 +932,22 @@ int terminal_maybe_give_to_job_group(const job_group_t *jg, bool continuing_from
/// Returns control of the terminal to the shell, and saves the terminal attribute state to the job /// Returns control of the terminal to the shell, and saves the terminal attribute state to the job
/// group, so that we can restore the terminal ownership to the job at a later time. /// group, so that we can restore the terminal ownership to the job at a later time.
static bool terminal_return_from_job_group(job_group_t *jg) { static void terminal_return_from_job_group(job_group_t *jg) {
errno = 0; errno = 0;
auto pgid = jg->get_pgid(); FLOG(proc_pgroup, "fish reclaiming terminal");
if (!pgid.has_value()) {
FLOG(proc_pgroup, "terminal_return_from_job() returning early due to no process group");
return true;
}
FLOG(proc_pgroup, "fish reclaiming terminal after job pgid", *pgid);
if (tcsetpgrp(STDIN_FILENO, getpgrp()) == -1) { if (tcsetpgrp(STDIN_FILENO, getpgrp()) == -1) {
if (errno == ENOTTY) redirect_tty_output();
FLOGF(warning, _(L"Could not return shell to foreground")); FLOGF(warning, _(L"Could not return shell to foreground"));
wperror(L"tcsetpgrp"); wperror(L"tcsetpgrp");
return false; return;
} }
// Save jobs terminal modes. // Save jobs terminal modes.
struct termios tmodes {}; struct termios tmodes {};
if (tcgetattr(STDIN_FILENO, &tmodes)) { if (tcgetattr(STDIN_FILENO, &tmodes) == 0) {
// If it's not a tty, it's not a tty, and there are no attributes to save (or restore) jg->tmodes = tmodes;
if (errno == ENOTTY) return false; } else if (errno != ENOTTY) {
FLOGF(warning, _(L"Could not return shell to foreground"));
wperror(L"tcgetattr"); wperror(L"tcgetattr");
return false;
} }
jg->tmodes = tmodes;
return true;
} }
bool job_t::is_foreground() const { return group->is_foreground(); } bool job_t::is_foreground() const { return group->is_foreground(); }
@@ -997,7 +962,7 @@ maybe_t<pid_t> job_t::get_last_pid() const {
return none(); return none();
} }
job_id_t job_t::job_id() const { return group->get_id(); } job_id_t job_t::job_id() const { return group->get_job_id(); }
void job_t::continue_job(parser_t &parser, bool in_foreground) { void job_t::continue_job(parser_t &parser, bool in_foreground) {
// Put job first in the job list. // Put job first in the job list.

18
src/proc.h
View File

@@ -278,6 +278,10 @@ class process_t : noncopyable_t {
/// True if process has stopped. /// True if process has stopped.
bool stopped{false}; bool stopped{false};
/// If set, this process is (or will become) the pgroup leader.
/// This is only meaningful for external processes.
bool leads_pgrp{false};
/// Reported status value. /// Reported status value.
proc_status_t status{}; proc_status_t status{};
@@ -318,9 +322,6 @@ class job_t : noncopyable_t {
/// Whether this job was created as part of an event handler. /// Whether this job was created as part of an event handler.
bool from_event_handler{}; bool from_event_handler{};
/// Whether the job is under job control, i.e. has its own pgrp.
bool job_control{};
}; };
private: private:
@@ -376,9 +377,8 @@ class job_t : noncopyable_t {
// This is never null and not changed after construction. // This is never null and not changed after construction.
job_group_ref_t group{}; job_group_ref_t group{};
/// \return the pgid for the job, based on the job group. /// \return our pgid, or none if we don't have one, or are internal to fish
/// This may be none if the job consists of just internal fish functions or builtins. /// This never returns fish's own pgroup.
/// This may also be fish itself.
maybe_t<pid_t> get_pgid() const; maybe_t<pid_t> get_pgid() const;
/// \return the pid of the last external process in the job. /// \return the pid of the last external process in the job.
@@ -424,7 +424,7 @@ class job_t : noncopyable_t {
bool wants_timing() const { return properties.wants_timing; } bool wants_timing() const { return properties.wants_timing; }
/// \return if we want job control. /// \return if we want job control.
bool wants_job_control() const { return properties.job_control; } bool wants_job_control() const;
/// \return whether this job is initially going to run in the background, because & was /// \return whether this job is initially going to run in the background, because & was
/// specified. /// specified.
@@ -439,6 +439,10 @@ class job_t : noncopyable_t {
bool has_internal_proc() const; bool has_internal_proc() const;
bool has_external_proc() const; bool has_external_proc() const;
/// \return whether this job, when run, will want a job ID.
/// Jobs that are only a single internal block do not get a job ID.
bool wants_job_id() const;
// Helper functions to check presence of flags on instances of jobs // 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 /// The job has been fully constructed, i.e. all its member processes have been launched
bool is_constructed() const { return flags().constructed; } bool is_constructed() const { return flags().constructed; }