From f3736e8fdf77652231527383ebbd9d96e71cec4d Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Wed, 26 Jun 2019 17:20:37 -0700 Subject: [PATCH] fish to claim a job's pgroup if the first process is fish internal When executing a job, if the first process is fish internal, then have fish claim the job's pgroup. The idea here is that the terminal must be owned by a pgroup containing the process reading from the terminal. If the first process is fish internal (a function or builtin) then the pgroup must contain the fish process. This is a bit of a workaround of the behavior where the first process that executes in a job becomes the process group leader. If there's a deferred process, then we will execute processes out of order so the pgroup can be wrong. Fix this by setting the process group leader explicitly as fish when necessary. Fixes #5855 --- src/exec.cpp | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/exec.cpp b/src/exec.cpp index 04a2cd2ee..01c926ad9 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1005,6 +1005,41 @@ static process_t *get_deferred_process(const shared_ptr &j) { return nullptr; } +/// \return true if fish should claim the process group for this job. +/// This is true if there is at least one external process and if the first process is fish code. +static bool should_claim_process_group_for_job(const shared_ptr &j) { + // Check if there's an external process. + // This is because historically fish has not reported job exits for internal-only processes, + // which is determined by comparing the pgrp against INVALID_PID. + bool has_external = false; + for (const auto &p : j->processes) { + if (p->type == process_type_t::external) { + has_external = true; + break; + } + } + if (!has_external) { + return false; + } + + // Check the first process. + // The terminal owner has to be the process which is permitted to read from stdin. + // This is the first process in the pipeline. When executing, a process in the job will + // claim the pgrp if it's not set; therefore set it according to the first process. + switch (j->processes.front()->type) { + case process_type_t::builtin: + case process_type_t::function: + case process_type_t::block_node: + // These are run internal to fish. + return true; + case process_type_t::external: + case process_type_t::exec: + // External will get its own pgroup after fork. + // exec will retain the pgroup. + return false; + } +} + bool exec_job(parser_t &parser, shared_ptr j) { assert(j && "null job_t passed to exec_job!"); @@ -1028,6 +1063,10 @@ bool exec_job(parser_t &parser, shared_ptr j) { j->set_flag(job_flag_t::JOB_CONTROL, true); } + if (j->pgid == INVALID_PID && should_claim_process_group_for_job(j)) { + j->pgid = getpgrp(); + } + size_t stdout_read_limit = 0; io_chain_t all_ios = j->all_io_redirections();