mirror of
https://github.com/fish-shell/fish-shell
synced 2024-12-26 21:03:12 +00:00
Remove NESTED and WAIT_BY_PROCESS
Now jobs are aware of their parent jobs, and can interrogate those jobs, to determine if every job in the chain is fully constructed. Remove flags and the static stacks that manipulated them.
This commit is contained in:
parent
30990e8069
commit
73537fc7c3
3 changed files with 3 additions and 56 deletions
50
src/exec.cpp
50
src/exec.cpp
|
@ -1002,44 +1002,13 @@ void exec_job(parser_t &parser, shared_ptr<job_t> j) {
|
|||
return;
|
||||
}
|
||||
|
||||
// Unfortunately `exec_job()` is called recursively when functions are encountered, with a new
|
||||
// job id (and therefore pgrp) each time, but always from the main thread. This breaks terminal
|
||||
// control since new pgrps take terminal control away from commands upstream in a different pgrp.
|
||||
// We try to work around this with a heuristic to determine whether to reuse the same pgrp as the
|
||||
// last-spawned pgrp if part of an existing job pipeline (keeping in mind that new jobs are
|
||||
// recursively started for both foreground and background jobs, and that a function can expand
|
||||
// to more than one external command, one (or more?) of which may want to read from upstream or
|
||||
// write to downstream of a pipe.
|
||||
// By keeping track of (select) "jobs in flight" we can try to marshall newly-created external
|
||||
// processes into existing pgrps. Fixes #3952.
|
||||
// This is a HACK and the correct solution would be to pass the active job through the pipeline
|
||||
// to the newly established parser context so that the funtion as parsed and evaluated can be
|
||||
// directly associated with this job and not a new one, BUT sometimes functions NEED to start a
|
||||
// new job. This HACK seeks a compromise by letting functions trigger the unilateral creation of
|
||||
// a new job, but reusing the "parent" job's existing pgrp in case of terminal control.
|
||||
static std::stack<decltype(j)> active_jobs;
|
||||
// There's an assumption that there's a one-to-one mapping between jobs under job control and
|
||||
// pgrps. When we share a parent job's pgrp, we risk reaping its processes before it is fully
|
||||
// constructed, causing later setpgrp(2) calls to fails (#5219). While the parent job is still
|
||||
// under construction, child jobs have job_flag_t::WAIT_BY_PROCESS set to prevent early repaing.
|
||||
// We store them here until the parent job is constructed, at which point it unsets this flag.
|
||||
static std::stack<decltype(j)> child_jobs;
|
||||
|
||||
auto parent_job = active_jobs.empty() ? nullptr : active_jobs.top();
|
||||
bool job_pushed = false;
|
||||
if (j->get_flag(job_flag_t::TERMINAL) && j->get_flag(job_flag_t::JOB_CONTROL)) {
|
||||
// This will be popped before this job leaves exec_job
|
||||
active_jobs.push(j);
|
||||
job_pushed = true;
|
||||
}
|
||||
const std::shared_ptr<job_t> parent_job = j->get_parent();
|
||||
|
||||
// Perhaps inherit our parent's pgid and job control flag.
|
||||
if (parent_job && j->processes.front()->type == EXTERNAL) {
|
||||
if (parent_job->pgid != INVALID_PID) {
|
||||
j->pgid = parent_job->pgid;
|
||||
j->set_flag(job_flag_t::JOB_CONTROL, true);
|
||||
j->set_flag(job_flag_t::NESTED, true);
|
||||
j->set_flag(job_flag_t::WAIT_BY_PROCESS, true);
|
||||
child_jobs.push(j);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1108,21 +1077,6 @@ void exec_job(parser_t &parser, shared_ptr<job_t> j) {
|
|||
env_set_one(L"last_pid", ENV_GLOBAL, to_string(j->pgid));
|
||||
}
|
||||
|
||||
if (job_pushed) {
|
||||
active_jobs.pop();
|
||||
}
|
||||
|
||||
if (!parent_job) {
|
||||
// Unset WAIT_BY_PROCESS on all child jobs. We could leave it, but this speeds up the
|
||||
// execution of `process_mark_finished_children()`.
|
||||
while (!child_jobs.empty()) {
|
||||
auto child = child_jobs.top();
|
||||
child_jobs.pop();
|
||||
|
||||
child->set_flag(job_flag_t::WAIT_BY_PROCESS, false);
|
||||
}
|
||||
}
|
||||
|
||||
if (!exec_error) {
|
||||
j->continue_job(false);
|
||||
} else {
|
||||
|
|
|
@ -1090,7 +1090,7 @@ void job_t::continue_job(bool send_sigcont) {
|
|||
// is also the only place that send_sigcont is false. parent_job.is_constructed()
|
||||
// must also be true, which coincides with WAIT_BY_PROCESS (which will have to do
|
||||
// since we don't store a reference to the parent job in the job_t structure).
|
||||
bool block_on_fg = send_sigcont && !get_flag(job_flag_t::WAIT_BY_PROCESS);
|
||||
bool block_on_fg = send_sigcont && job_chain_is_fully_constructed();
|
||||
|
||||
// Wait for data to become available or the status of our own job to change
|
||||
while (!reader_exit_forced() && !is_stopped() && !is_completed()) {
|
||||
|
|
|
@ -157,13 +157,6 @@ enum class job_flag_t {
|
|||
JOB_CONTROL,
|
||||
/// Whether the job wants to own the terminal when in the foreground.
|
||||
TERMINAL,
|
||||
/// This job was created via a recursive call to exec_job upon evaluation of a function.
|
||||
/// Ideally it should not be a top-level job, and there are places where it won't be treated
|
||||
/// as such.
|
||||
NESTED,
|
||||
/// This job shares a pgrp with a not-yet-constructed job, so we can't waitpid on its pgid
|
||||
/// directly. Hack to work around fucntions starting new jobs. See `exec_job()`.
|
||||
WAIT_BY_PROCESS
|
||||
};
|
||||
|
||||
typedef int job_id_t;
|
||||
|
|
Loading…
Reference in a new issue