From 39a974099795f532536c4fc821fa18a5379e3db3 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 6 Apr 2019 20:00:52 -0700 Subject: [PATCH] Be less aggressive about reclaiming the foreground pgrp Prior to this fix, in every call to job_continue, fish would reclaim the foreground pgrp. This would cause other jobs in the pipeline (which may have another pgrp) to receive SIGTTIN / SIGTTOU. Only reclaim the foreground pgrp if it was held at the point of job_continue. This partially addresses #5765 --- src/builtin_bg.cpp | 2 +- src/builtin_fg.cpp | 2 +- src/exec.cpp | 5 ++++- src/proc.cpp | 4 ++-- src/proc.h | 6 ++++-- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/builtin_bg.cpp b/src/builtin_bg.cpp index e737edae4..f8ee41558 100644 --- a/src/builtin_bg.cpp +++ b/src/builtin_bg.cpp @@ -30,7 +30,7 @@ static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) { j->command_wcstr()); j->promote(); j->set_flag(job_flag_t::FOREGROUND, false); - j->continue_job(j->is_stopped()); + j->continue_job(true, j->is_stopped()); return STATUS_CMD_OK; } diff --git a/src/builtin_fg.cpp b/src/builtin_fg.cpp index 5bd9221b6..7ce712bd7 100644 --- a/src/builtin_fg.cpp +++ b/src/builtin_fg.cpp @@ -106,6 +106,6 @@ int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) { job->promote(); job->set_flag(job_flag_t::FOREGROUND, true); - job->continue_job(job->is_stopped()); + job->continue_job(true, job->is_stopped()); return STATUS_CMD_OK; } diff --git a/src/exec.cpp b/src/exec.cpp index 7fc37af5e..375f0e44f 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1021,6 +1021,9 @@ bool exec_job(parser_t &parser, shared_ptr j) { return true; } + // Check to see if we should reclaim the foreground pgrp after the job finishes or stops. + const bool reclaim_foreground_pgrp = (tcgetpgrp(STDIN_FILENO) == getpgrp()); + const std::shared_ptr parent_job = j->get_parent(); // Perhaps inherit our parent's pgid and job control flag. @@ -1120,7 +1123,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { return false; } - j->continue_job(false); + j->continue_job(reclaim_foreground_pgrp, false); return true; } diff --git a/src/proc.cpp b/src/proc.cpp index 169116abd..514459af0 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -827,7 +827,7 @@ return false; return true; } -void job_t::continue_job(bool send_sigcont) { +void job_t::continue_job(bool reclaim_foreground_pgrp, bool send_sigcont) { // Put job first in the job list. promote(); set_flag(job_flag_t::NOTIFIED, false); @@ -839,7 +839,7 @@ void job_t::continue_job(bool send_sigcont) { // Make sure we retake control of the terminal before leaving this function. bool term_transferred = false; cleanup_t take_term_back([&]() { - if (term_transferred) { + if (term_transferred && reclaim_foreground_pgrp) { terminal_return_from_job(this); } }); diff --git a/src/proc.h b/src/proc.h index e7106dabe..351a30740 100644 --- a/src/proc.h +++ b/src/proc.h @@ -408,8 +408,10 @@ class job_t { /// saved terminal modes and send the process group a SIGCONT signal to wake it up before we /// block. /// - /// \param send_sigcont Whether SIGCONT should be sent to the job if it is in the foreground. - void continue_job(bool send_sigcont); + /// \param reclaim_foreground_pgrp whether, when the job finishes or stops, to reclaim the + /// foreground pgrp (via tcsetpgrp). \param send_sigcont Whether SIGCONT should be sent to the + /// job if it is in the foreground. + void continue_job(bool reclaim_foreground_pgrp, bool send_sigcont); /// Promotes the job to the front of the job list. void promote();