From f5bb8639d61bebb34f6937cffe0b4bf3e399f867 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 7 Apr 2019 13:08:37 -0700 Subject: [PATCH] More aggressively inherit pgrps from parent jobs Prior to this fix, a job would only inherit a pgrp from its parent if the first command were external. There seems to be no reason for this restriction and this causes tcsetgrp() churn, potentially cuasing SIGTTIN. Switch to unconditionally inheriting a pgrp from parents. This should fix most of #5765, the only remaining question is tcsetpgrp from builtins. --- src/exec.cpp | 8 +++----- tests/pipeline.expect | 10 ++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index 375f0e44f..be1eec6b6 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1027,11 +1027,9 @@ bool exec_job(parser_t &parser, shared_ptr j) { const std::shared_ptr parent_job = j->get_parent(); // Perhaps inherit our parent's pgid and job control flag. - if (parent_job && j->processes.front()->type == process_type_t::external) { - if (parent_job->pgid != INVALID_PID) { - j->pgid = parent_job->pgid; - j->set_flag(job_flag_t::JOB_CONTROL, true); - } + if (parent_job && parent_job->pgid != INVALID_PID) { + j->pgid = parent_job->pgid; + j->set_flag(job_flag_t::JOB_CONTROL, true); } size_t stdout_read_limit = 0; diff --git a/tests/pipeline.expect b/tests/pipeline.expect index 586e78d98..eff55e9e4 100644 --- a/tests/pipeline.expect +++ b/tests/pipeline.expect @@ -14,3 +14,13 @@ for {set x 0} {$x<15} {incr x} { # 'not' because we expect to have no jobs, in which case `jobs` will return false send_line "not jobs" expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr "Should be no jobs" } + +send_line "function inner ; command true ; end; function outer; inner; end" +expect_prompt +for {set x 0} {$x<15} {incr x} { + send_line "outer | $fish_test_helper become_foreground_then_print_stderr ; or exit 1" + expect_prompt "become_foreground_then_print_stderr done" {} unmatched { puts stderr "fish_test_helper become_foreground_then_print_stderr failed"} +} + +send_line "not jobs" +expect_prompt "jobs: There are no jobs" {} unmatched { puts stderr "Should be no jobs" }