diff --git a/src/exec.cpp b/src/exec.cpp index dd10c5ec3..1b246bd8a 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1013,20 +1013,6 @@ static process_t *get_deferred_process(const shared_ptr &j) { /// \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 diff --git a/src/proc.cpp b/src/proc.cpp index d945bac0d..70c0adb6d 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -148,6 +148,25 @@ bool job_t::is_completed() const { return true; } +bool job_t::should_report_process_exits() const { + // This implements the behavior of process exit events only being sent for jobs containing an + // external process. Bizarrely the process exit event is for the pgroup leader which may be fish + // itself. + // TODO: rationalize this. + // If we never got a pgid then we never launched the external process, so don't report it. + if (this->pgid == INVALID_PID) { + return false; + } + + // Return whether we have an external process. + for (const auto &p : this->processes) { + if (p->type == process_type_t::external) { + return true; + } + } + return false; +} + bool job_t::job_chain_is_fully_constructed() const { const job_t *cursor = this; while (cursor) { @@ -585,7 +604,7 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive // Prepare events for completed jobs, except for jobs that themselves came from event // handlers. if (!j->from_event_handler() && j->is_completed()) { - if (j->pgid != INVALID_PID) { + if (j->should_report_process_exits()) { exit_events.push_back( proc_create_event(L"JOB_EXIT", event_type_t::exit, -j->pgid, 0)); } diff --git a/src/proc.h b/src/proc.h index 934d7b48e..d3d3e6455 100644 --- a/src/proc.h +++ b/src/proc.h @@ -423,6 +423,10 @@ class job_t { bool skip_notification() const { return properties.skip_notification; } bool from_event_handler() const { return properties.from_event_handler; } + /// \return whether we should report process exit events. + /// This implements some historical behavior which has not been justified. + bool should_report_process_exits() const; + /// \return the parent job, or nullptr. const std::shared_ptr get_parent() const { return parent_job; }