Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-31 23:28:45 +00:00
Code Issues Projects Releases Packages Wiki Activity

Refactor process_mark_finished_children

Browse source 
Reduce the level of nesting and the loop complexity.
This commit is contained in:
ridiculousfish 2020-08-07 12:33:43 -07:00
parent 203061292f
commit 2cd336376e
2 changed files with 65 additions and 62 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

89
src/proc.cpp
View file

@ -387,12 +387,20 @@ static void process_mark_finished_children(parser_t &parser, bool block_ok) {
// Get the exit and signal generations of all reapable processes. // Get the exit and signal generations of all reapable processes.
// The exit generation tells us if we have an exit; the signal generation allows for detecting // The exit generation tells us if we have an exit; the signal generation allows for detecting
// SIGHUP and SIGINT. // SIGHUP and SIGINT.
// Get the gen count of all reapable processes. // Go through each process and figure out if and how it wants to be reaped.
generation_list_t reapgens = generation_list_t::invalids(); generation_list_t reapgens = generation_list_t::invalids();
for (const auto &j : parser.jobs()) { for (const auto &j : parser.jobs()) {
for (const auto &proc : j->processes) { for (const auto &proc : j->processes) {
if (auto mtopic = j->reap_topic_for_process(proc.get())) { if (!j->can_reap(proc)) continue;
reapgens.set_min_from(*mtopic, proc->gens_);
if (proc->pid) {
// Reaps with a pid.
reapgens.set_min_from(topic_t::sigchld, proc->gens_);
reapgens.set_min_from(topic_t::sighupint, proc->gens_);
}
if (proc->internal_proc_) {
// Reaps with an internal process.
reapgens.set_min_from(topic_t::internal_exit, proc->gens_);
reapgens.set_min_from(topic_t::sighupint, proc->gens_); reapgens.set_min_from(topic_t::sighupint, proc->gens_);
} }
} }
@ -406,55 +414,60 @@ static void process_mark_finished_children(parser_t &parser, bool block_ok) {
// We got some changes. Since we last checked we received SIGCHLD, and or HUP/INT. // We got some changes. Since we last checked we received SIGCHLD, and or HUP/INT.
// Update the hup/int generations and reap any reapable processes. // Update the hup/int generations and reap any reapable processes.
for (auto &j : parser.jobs()) { // We structure this as two loops for some simplicity.
// First reap all pids.
for (const auto &j : parser.jobs()) {
for (const auto &proc : j->processes) { for (const auto &proc : j->processes) {
if (auto mtopic = j->reap_topic_for_process(proc.get())) { // Does this proc have a pid that is reapable?
topic_t topic = *mtopic; if (proc->pid <= 0 || !j->can_reap(proc)) continue;
// Update the signal hup/int gen. // Update the signal hup/int gen.
proc->gens_.sighupint = reapgens.sighupint; proc->gens_.sighupint = reapgens.sighupint;
if (proc->gens_.at(topic) < reapgens.at(topic)) { // Nothing to do if we did not get a new sigchld.
// Potentially reapable. Update its generation and try reaping it. if (proc->gens_.sigchld == reapgens.sigchld) continue;
proc->gens_.at(topic) = reapgens.at(topic); proc->gens_.sigchld = reapgens.sigchld;
if (proc->internal_proc_) {
// Try reaping an internal process. // Ok, we are reapable. Run waitpid()!
if (proc->internal_proc_->exited()) { int statusv = -1;
handle_child_status(j, proc.get(), proc->internal_proc_->get_status()); pid_t pid = waitpid(proc->pid, &statusv, WNOHANG | WUNTRACED | WCONTINUED);
FLOGF(proc_reap_internal, assert(pid <= 0 || pid == proc->pid && "Unexpcted waitpid() return");
"Reaped internal process '%ls' (id %llu, status %d)", if (pid <= 0) continue;
proc->argv0(), proc->internal_proc_->get_id(),
proc->status.status_value()); // The process has stopped or exited! Update its status.
} proc_status_t status = proc_status_t::from_waitpid(statusv);
} else if (proc->pid > 0) { handle_child_status(j, proc.get(), status);
// Try reaping an external process. if (status.stopped()) {
int status = -1;
auto pid = waitpid(proc->pid, &status, WNOHANG | WUNTRACED | WCONTINUED);
if (pid > 0) {
assert(pid == proc->pid && "Unexpcted waitpid() return");
handle_child_status(j, proc.get(), proc_status_t::from_waitpid(status));
if (proc->status.stopped()) {
j->group->set_is_foreground(false); j->group->set_is_foreground(false);
} }
if (proc->status.continued()) { if (status.continued()) {
j->mut_flags().notified = false; j->mut_flags().notified = false;
} }
if (proc->status.normal_exited() || proc->status.signal_exited()) { if (status.normal_exited() || status.signal_exited()) {
FLOGF(proc_reap_external, FLOGF(proc_reap_external, "Reaped external process '%ls' (pid %d, status %d)",
"Reaped external process '%ls' (pid %d, status %d)",
proc->argv0(), pid, proc->status.status_value()); proc->argv0(), pid, proc->status.status_value());
} else { } else {
assert(proc->status.stopped() || proc->status.continued()); assert(status.stopped() || status.continued());
FLOGF(proc_reap_external, "External process '%ls' (pid %d, %s)", FLOGF(proc_reap_external, "External process '%ls' (pid %d, %s)", proc->argv0(),
proc->argv0(), pid, proc->pid, proc->status.stopped() ? "stopped" : "continued");
proc->status.stopped() ? "stopped" : "continued");
}
}
} else {
assert(0 && "Don't know how to reap this process");
} }
} }
} }
// We are done reaping pids.
// Reap internal processes.
for (const auto &j : parser.jobs()) {
for (const auto &proc : j->processes) {
// Does this proc have an internal process that is reapable?
if (!proc->internal_proc_ || !j->can_reap(proc)) continue;
// Has the process exited?
if (!proc->internal_proc_->exited()) continue;
// The process gets the status from its internal proc.
handle_child_status(j, proc.get(), proc->internal_proc_->get_status());
FLOGF(proc_reap_internal, "Reaped internal process '%ls' (id %llu, status %d)",
proc->argv0(), proc->internal_proc_->get_id(), proc->status.status_value());
} }
} }

18
src/proc.h
View file

@ -350,14 +350,11 @@ class job_t {
/// \return whether it is OK to reap a given process. Sometimes we want to defer reaping a /// \return whether it is OK to reap a given process. Sometimes we want to defer reaping a
/// process if it is the group leader and the job is not yet constructed, because then we might /// process if it is the group leader and the job is not yet constructed, because then we might
/// also reap the process group and then we cannot add new processes to the group. /// also reap the process group and then we cannot add new processes to the group.
bool can_reap(const process_t *p) const { bool can_reap(const process_ptr_t &p) const {
// Internal processes can always be reaped. if (p->completed) {
if (p->internal_proc_) { // Can't reap twice.
return true;
} else if (p->pid <= 0) {
// Can't reap without a pid.
return false; return false;
} else if (!is_constructed() && this->get_pgid() == maybe_t<pid_t>{p->pid}) { } else if (p->pid && !is_constructed() && this->get_pgid() == maybe_t<pid_t>{p->pid}) {
// p is the the group leader in an under-construction job. // p is the the group leader in an under-construction job.
return false; return false;
} else { } else {
@ -365,13 +362,6 @@ class job_t {
} }
} }
/// \returns the reap topic for a process, which describes the manner in which we are reaped. A
/// none returns means don't reap, or perhaps defer reaping.
maybe_t<topic_t> reap_topic_for_process(const process_t *p) const {
if (p->completed || !can_reap(p)) return none();
return p->internal_proc_ ? topic_t::internal_exit : topic_t::sigchld;
}
/// Returns a truncated version of the job string. Used when a message has already been emitted /// Returns a truncated version of the job string. Used when a message has already been emitted
/// containing the full job string and job id, but using the job id alone would be confusing /// containing the full job string and job id, but using the job id alone would be confusing
/// due to reuse of freed job ids. Prevents overloading the debug comments with the full, /// due to reuse of freed job ids. Prevents overloading the debug comments with the full,