Factor job groups into their own file

Migrate out of proc.h, which has become too long.

CMakeLists.txt

@@ -113,7 +113,7 @@ set(FISH_SRCS
     src/expand.cpp src/fallback.cpp src/fd_monitor.cpp src/fish_version.cpp
     src/flog.cpp src/function.cpp src/future_feature_flags.cpp src/highlight.cpp
     src/history.cpp src/history_file.cpp src/input.cpp src/input_common.cpp
-    src/intern.cpp src/io.cpp src/iothread.cpp src/kill.cpp
+    src/intern.cpp src/io.cpp src/iothread.cpp src/job_group.cpp src/kill.cpp
     src/null_terminated_array.cpp src/operation_context.cpp src/output.cpp
     src/pager.cpp src/parse_execution.cpp src/parse_tree.cpp src/parse_util.cpp
     src/parser.cpp src/parser_keywords.cpp src/path.cpp src/postfork.cpp

src/builtin_bg.cpp

@@ -12,6 +12,7 @@
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "io.h"
+#include "job_group.h"
 #include "parser.h"
 #include "proc.h"
 #include "wutil.h"  // IWYU pragma: keep

src/builtin_fg.cpp

@@ -13,6 +13,7 @@
 #include "env.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "io.h"
+#include "job_group.h"
 #include "parser.h"
 #include "proc.h"
 #include "reader.h"

src/exec.cpp

@@ -36,6 +36,7 @@
 #include "function.h"
 #include "io.h"
 #include "iothread.h"
+#include "job_group.h"
 #include "null_terminated_array.h"
 #include "parse_tree.h"
 #include "parser.h"

src/job_group.cpp

@@ -0,0 +1,99 @@
+#include "config.h"
+
+#include "job_group.h"
+
+#include "common.h"
+#include "fallback.h"  // IWYU pragma: keep
+#include "flog.h"
+#include "proc.h"
+
+// Basic thread safe sorted vector of job IDs in use.
+// This is deliberately leaked to avoid dtor ordering issues - see #6539.
+static const auto locked_consumed_job_ids = new owning_lock<std::vector<job_id_t>>();
+
+static job_id_t acquire_job_id() {
+    auto consumed_job_ids = locked_consumed_job_ids->acquire();
+
+    // The new job ID should be larger than the largest currently used ID (#6053).
+    job_id_t jid = consumed_job_ids->empty() ? 1 : consumed_job_ids->back() + 1;
+    consumed_job_ids->push_back(jid);
+    return jid;
+}
+
+static void release_job_id(job_id_t jid) {
+    assert(jid > 0);
+    auto consumed_job_ids = locked_consumed_job_ids->acquire();
+
+    // Our job ID vector is sorted, but the number of jobs is typically 1 or 2 so a binary search
+    // isn't worth it.
+    auto where = std::find(consumed_job_ids->begin(), consumed_job_ids->end(), jid);
+    assert(where != consumed_job_ids->end() && "Job ID was not in use");
+    consumed_job_ids->erase(where);
+}
+
+job_group_t::~job_group_t() {
+    if (props_.job_id > 0) {
+        release_job_id(props_.job_id);
+    }
+}
+
+void job_group_t::set_pgid(pid_t pgid) {
+    // Note we need not be concerned about thread safety. job_groups are intended to be shared
+    // across threads, but their pgid should always have been set beforehand.
+    assert(needs_pgid_assignment() && "We should not be setting a pgid");
+    assert(pgid >= 0 && "Invalid pgid");
+    pgid_ = pgid;
+}
+
+maybe_t<pid_t> job_group_t::get_pgid() const { return pgid_; }
+
+void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &proposed) {
+    assert(!job->group && "Job already has a group");
+    // Note there's three cases to consider:
+    //  nullptr         -> this is a root job, there is no inherited job group
+    //  internal        -> the parent is running as part of a simple function execution
+    //                      We may need to create a new job group if we are going to fork.
+    //  non-internal    -> we are running as part of a real pipeline
+    // Decide if this job can use an internal group.
+    // This is true if it's a simple foreground execution of an internal proc.
+    bool initial_bg = job->is_initially_background();
+    bool first_proc_internal = job->processes.front()->is_internal();
+    bool can_use_internal =
+        !initial_bg && job->processes.size() == 1 && job->processes.front()->is_internal();
+
+    bool needs_new_group = false;
+    if (!proposed) {
+        // We don't have a group yet.
+        needs_new_group = true;
+    } else if (initial_bg) {
+        // Background jobs always get a new group.
+        needs_new_group = true;
+    } else if (proposed->is_internal() && !can_use_internal) {
+        // We cannot use the internal group for this job.
+        needs_new_group = true;
+    }
+
+    job->mut_flags().is_group_root = needs_new_group;
+
+    if (!needs_new_group) {
+        job->group = proposed;
+    } else {
+        properties_t props{};
+        props.job_control = job->wants_job_control();
+        props.wants_terminal = job->wants_job_control() && !job->from_event_handler();
+        props.is_internal = can_use_internal;
+        props.job_id = can_use_internal ? -1 : acquire_job_id();
+        job->group.reset(new job_group_t(props, job->command()));
+
+        // Mark if it's foreground.
+        job->group->set_is_foreground(!initial_bg);
+
+        // Perhaps this job should immediately live in fish's pgroup.
+        // There's two reasons why it may be so:
+        //  1. The job doesn't need job control.
+        //  2. The first process in the job is internal to fish; this needs to own the tty.
+        if (!can_use_internal && (!props.job_control || first_proc_internal)) {
+            job->group->set_pgid(getpgrp());
+        }
+    }
+}

src/job_group.h

@@ -0,0 +1,125 @@
+#ifndef FISH_JOB_GROUP_H
+#define FISH_JOB_GROUP_H
+#include "config.h"  // IWYU pragma: keep
+
+#include <termios.h>
+
+#include <memory>
+
+#include "common.h"
+#include "global_safety.h"
+
+/// A job ID, corresponding to what is printed in 'jobs'.
+/// 1 is the first valid job ID.
+using job_id_t = int;
+
+/// job_group_t is conceptually similar to the idea of a process group. It represents data which
+/// is shared among all of the "subjobs" that may be spawned by a single job.
+/// For example, two fish functions in a pipeline may themselves spawn multiple jobs, but all will
+/// share the same job group.
+/// There is also a notion of a "internal" job group. Internal groups are used when executing a
+/// foreground function or block with no pipeline. These are not jobs as the user understands them -
+/// they do not consume a job ID, they do not show up in job lists, and they do not have a pgid
+/// because they contain no external procs. Note that job_group_t is intended to eventually be
+/// shared between threads, and so must be thread safe.
+class job_t;
+class job_group_t;
+using job_group_ref_t = std::shared_ptr<job_group_t>;
+
+class job_group_t {
+   public:
+    /// Set the pgid for this job group, latching it to this value.
+    /// The pgid should not already have been set.
+    /// Of course this does not keep the pgid alive by itself.
+    /// An internal job group does not have a pgid and it is an error to set it.
+    void set_pgid(pid_t pgid);
+
+    /// Get the pgid, or none() if it has not been set.
+    maybe_t<pid_t> get_pgid() const;
+
+    /// \return whether we want job control
+    bool wants_job_control() const { return props_.job_control; }
+
+    /// \return whether this is an internal group.
+    bool is_internal() const { return props_.is_internal; }
+
+    /// \return whether we are currently the foreground group.
+    bool is_foreground() const { return is_foreground_; }
+
+    /// Mark whether we are in the foreground.
+    void set_is_foreground(bool flag) { is_foreground_ = flag; }
+
+    /// \return if this job group should own the terminal when it runs.
+    bool should_claim_terminal() const { return props_.wants_terminal && is_foreground(); }
+
+    /// \return whether this job group is awaiting a pgid.
+    /// This is true for non-internal trees that don't already have a pgid.
+    bool needs_pgid_assignment() const { return !props_.is_internal && !pgid_.has_value(); }
+
+    /// \return the job ID, or -1 if none.
+    job_id_t get_id() const { return props_.job_id; }
+
+    /// Get the cancel signal, or 0 if none.
+    int get_cancel_signal() const { return cancel_signal_; }
+
+    /// \return the command which produced this job tree.
+    const wcstring &get_command() const { return command_; }
+
+    /// Mark that a process in this group got a signal, and so should cancel.
+    void set_cancel_signal(int sig) { cancel_signal_ = sig; }
+
+    /// Mark the root as constructed.
+    /// This is used to avoid reaping a process group leader while there are still procs that may
+    /// want to enter its group.
+    void mark_root_constructed() { root_constructed_ = true; };
+    bool is_root_constructed() const { return root_constructed_; }
+
+    /// Given a job and a proposed job group (possibly null), populate the job's group field.
+    /// The proposed group is the group from the parent job, or null if this is a root.
+    static void populate_group_for_job(job_t *job, const job_group_ref_t &proposed_tree);
+
+    ~job_group_t();
+
+    /// If set, the saved terminal modes of this job. This needs to be saved so that we can restore
+    /// the terminal to the same state after temporarily taking control over the terminal when a job
+    /// stops.
+    maybe_t<struct termios> tmodes{};
+
+   private:
+    // The pgid to assign to jobs, or none if not yet set.
+    maybe_t<pid_t> pgid_{};
+
+    // Set of properties, which are constant.
+    struct properties_t {
+        // Whether jobs in this group should have job control.
+        bool job_control{};
+
+        // Whether we should claim the terminal when we run in the foreground.
+        // TODO: this is effectively the same as job control, rationalize this.
+        bool wants_terminal{};
+
+        // Whether we are an internal job group.
+        bool is_internal{};
+
+        // The job ID of this group.
+        job_id_t job_id{};
+    };
+    const properties_t props_;
+
+    // The original command which produced this job tree.
+    const wcstring command_;
+
+    // Whether we are in the foreground, meaning that the user is waiting for this.
+    relaxed_atomic_bool_t is_foreground_{};
+
+    // Whether the root job is constructed. If not, we cannot reap it yet.
+    relaxed_atomic_bool_t root_constructed_{};
+
+    // If not zero, a signal indicating cancellation.
+    int cancel_signal_{};
+
+    job_group_t(const properties_t &props, wcstring command)
+        : props_(props), command_(std::move(command)) {}
+};
+
+#endif

src/parse_execution.cpp

@@ -30,6 +30,7 @@
 #include "flog.h"
 #include "function.h"
 #include "io.h"
+#include "job_group.h"
 #include "maybe.h"
 #include "parse_constants.h"
 #include "parse_util.h"

src/parser.cpp

@@ -20,6 +20,7 @@
 #include "flog.h"
 #include "function.h"
 #include "intern.h"
+#include "job_group.h"
 #include "parse_constants.h"
 #include "parse_execution.h"
 #include "parse_util.h"

src/postfork.cpp

@@ -19,6 +19,7 @@
 #include "flog.h"
 #include "io.h"
 #include "iothread.h"
+#include "job_group.h"
 #include "postfork.h"
 #include "proc.h"
 #include "redirection.h"

src/proc.cpp

@@ -43,6 +43,7 @@
 #include "flog.h"
 #include "global_safety.h"
 #include "io.h"
+#include "job_group.h"
 #include "output.h"
 #include "parse_tree.h"
 #include "parser.h"
@@ -94,30 +95,6 @@ void set_job_control_mode(job_control_t mode) {
 
 void proc_init() { signal_set_handlers_once(false); }
 
-// Basic thread safe sorted vector of job IDs in use.
-// This is deliberately leaked to avoid dtor ordering issues - see #6539.
-static const auto locked_consumed_job_ids = new owning_lock<std::vector<job_id_t>>();
-
-job_id_t acquire_job_id() {
-    auto consumed_job_ids = locked_consumed_job_ids->acquire();
-
-    // The new job ID should be larger than the largest currently used ID (#6053).
-    job_id_t jid = consumed_job_ids->empty() ? 1 : consumed_job_ids->back() + 1;
-    consumed_job_ids->push_back(jid);
-    return jid;
-}
-
-void release_job_id(job_id_t jid) {
-    assert(jid > 0);
-    auto consumed_job_ids = locked_consumed_job_ids->acquire();
-
-    // Our job ID vector is sorted, but the number of jobs is typically 1 or 2 so a binary search
-    // isn't worth it.
-    auto where = std::find(consumed_job_ids->begin(), consumed_job_ids->end(), jid);
-    assert(where != consumed_job_ids->end() && "JobID was not in use");
-    consumed_job_ids->erase(where);
-}
-
 /// Return true if all processes in the job have stopped or completed.
 bool job_t::is_stopped() const {
     for (const process_ptr_t &p : processes) {
@@ -241,73 +218,6 @@ void print_exit_warning_for_jobs(const job_list_t &jobs) {
     fputws(_(L"Use 'disown PID' to remove jobs from the list without terminating them.\n"), stdout);
 }
 
-job_group_t::~job_group_t() {
-    if (props_.job_id > 0) {
-        release_job_id(props_.job_id);
-    }
-}
-
-void job_group_t::set_pgid(pid_t pgid) {
-    // Note we need not be concerned about thread safety. job_groups are intended to be shared
-    // across threads, but their pgid should always have been set beforehand.
-    assert(needs_pgid_assignment() && "We should not be setting a pgid");
-    assert(pgid >= 0 && "Invalid pgid");
-    pgid_ = pgid;
-}
-
-maybe_t<pid_t> job_group_t::get_pgid() const { return pgid_; }
-
-void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &proposed) {
-    assert(!job->group && "Job already has a group");
-    // Note there's three cases to consider:
-    //  nullptr         -> this is a root job, there is no inherited job group
-    //  internal        -> the parent is running as part of a simple function execution
-    //                      We may need to create a new job group if we are going to fork.
-    //  non-internal    -> we are running as part of a real pipeline
-    // Decide if this job can use an internal group.
-    // This is true if it's a simple foreground execution of an internal proc.
-    bool initial_bg = job->is_initially_background();
-    bool first_proc_internal = job->processes.front()->is_internal();
-    bool can_use_internal =
-        !initial_bg && job->processes.size() == 1 && job->processes.front()->is_internal();
-
-    bool needs_new_group = false;
-    if (!proposed) {
-        // We don't have a group yet.
-        needs_new_group = true;
-    } else if (initial_bg) {
-        // Background jobs always get a new group.
-        needs_new_group = true;
-    } else if (proposed->is_internal() && !can_use_internal) {
-        // We cannot use the internal group for this job.
-        needs_new_group = true;
-    }
-
-    job->mut_flags().is_group_root = needs_new_group;
-
-    if (!needs_new_group) {
-        job->group = proposed;
-    } else {
-        properties_t props{};
-        props.job_control = job->wants_job_control();
-        props.wants_terminal = job->wants_job_control() && !job->from_event_handler();
-        props.is_internal = can_use_internal;
-        props.job_id = can_use_internal ? -1 : acquire_job_id();
-        job->group.reset(new job_group_t(props, job->command()));
-
-        // Mark if it's foreground.
-        job->group->set_is_foreground(!initial_bg);
-
-        // Perhaps this job should immediately live in fish's pgroup.
-        // There's two reasons why it may be so:
-        //  1. The job doesn't need job control.
-        //  2. The first process in the job is internal to fish; this needs to own the tty.
-        if (!can_use_internal && (!props.job_control || first_proc_internal)) {
-            job->group->set_pgid(getpgrp());
-        }
-    }
-}
-
 void job_mark_process_as_failed(const std::shared_ptr<job_t> &job, const process_t *failed_proc) {
     // The given process failed to even lift off (e.g. posix_spawn failed) and so doesn't have a
     // valid pid. Mark it and everything after it as dead.
@@ -963,6 +873,12 @@ static bool terminal_return_from_job_group(job_group_t *jg, bool restore_attrs)
     return true;
 }
 
+bool job_t::is_foreground() const { return group->is_foreground(); }
+
+maybe_t<pid_t> job_t::get_pgid() const { return group->get_pgid(); }
+
+job_id_t job_t::job_id() const { return group->get_id(); }
+
 void job_t::continue_job(parser_t &parser, bool reclaim_foreground_pgrp, bool send_sigcont) {
     // Put job first in the job list.
     parser.job_promote(this);

src/proc.h

@@ -9,7 +9,6 @@
 #include <stddef.h>
 #include <sys/time.h>  // IWYU pragma: keep
 #include <sys/wait.h>
-#include <termios.h>
 #include <unistd.h>
 
 #include <deque>
@@ -47,6 +46,9 @@ namespace ast {
 struct statement_t;
 }
 
+class job_group_t;
+using job_group_ref_t = std::shared_ptr<job_group_t>;
+
 /// A proc_status_t is a value type that encapsulates logic around exited vs stopped vs signaled,
 /// etc.
 class proc_status_t {
@@ -157,121 +159,6 @@ class internal_proc_t {
 ///   function
 enum { INVALID_PID = -2 };
 
-/// A job ID, corresponding to what is printed in 'jobs'.
-/// 1 is the first valid job ID.
-using job_id_t = int;
-job_id_t acquire_job_id(void);
-void release_job_id(job_id_t jid);
-
-/// job_group_t is conceptually similar to the idea of a process group. It represents data which
-/// is shared among all of the "subjobs" that may be spawned by a single job.
-/// For example, two fish functions in a pipeline may themselves spawn multiple jobs, but all will
-/// share the same job group.
-/// There is also a notion of a "internal" job group. Internal groups are used when executing a
-/// foreground function or block with no pipeline. These are not jobs as the user understands them -
-/// they do not consume a job ID, they do not show up in job lists, and they do not have a pgid
-/// because they contain no external procs. Note that job_group_t is intended to eventually be
-/// shared between threads, and so must be thread safe.
-class job_t;
-class job_group_t;
-using job_group_ref_t = std::shared_ptr<job_group_t>;
-
-class job_group_t {
-   public:
-    /// Set the pgid for this job group, latching it to this value.
-    /// The pgid should not already have been set.
-    /// Of course this does not keep the pgid alive by itself.
-    /// An internal job group does not have a pgid and it is an error to set it.
-    void set_pgid(pid_t pgid);
-
-    /// Get the pgid, or none() if it has not been set.
-    maybe_t<pid_t> get_pgid() const;
-
-    /// \return whether we want job control
-    bool wants_job_control() const { return props_.job_control; }
-
-    /// \return whether this is an internal group.
-    bool is_internal() const { return props_.is_internal; }
-
-    /// \return whether we are currently the foreground group.
-    bool is_foreground() const { return is_foreground_; }
-
-    /// Mark whether we are in the foreground.
-    void set_is_foreground(bool flag) { is_foreground_ = flag; }
-
-    /// \return if this job group should own the terminal when it runs.
-    bool should_claim_terminal() const { return props_.wants_terminal && is_foreground(); }
-
-    /// \return whether this job group is awaiting a pgid.
-    /// This is true for non-internal trees that don't already have a pgid.
-    bool needs_pgid_assignment() const { return !props_.is_internal && !pgid_.has_value(); }
-
-    /// \return the job ID, or -1 if none.
-    job_id_t get_id() const { return props_.job_id; }
-
-    /// Get the cancel signal, or 0 if none.
-    int get_cancel_signal() const { return cancel_signal_; }
-
-    /// \return the command which produced this job tree.
-    const wcstring &get_command() const { return command_; }
-
-    /// Mark that a process in this group got a signal, and so should cancel.
-    void set_cancel_signal(int sig) { cancel_signal_ = sig; }
-
-    /// Mark the root as constructed.
-    /// This is used to avoid reaping a process group leader while there are still procs that may
-    /// want to enter its group.
-    void mark_root_constructed() { root_constructed_ = true; };
-    bool is_root_constructed() const { return root_constructed_; }
-
-    /// Given a job and a proposed job group (possibly null), populate the job's group field.
-    /// The proposed group is the group from the parent job, or null if this is a root.
-    static void populate_group_for_job(job_t *job, const job_group_ref_t &proposed_tree);
-
-    ~job_group_t();
-
-    /// If set, the saved terminal modes of this job. This needs to be saved so that we can restore
-    /// the terminal to the same state after temporarily taking control over the terminal when a job
-    /// stops.
-    maybe_t<struct termios> tmodes{};
-
-   private:
-    // The pgid to assign to jobs, or none if not yet set.
-    maybe_t<pid_t> pgid_{};
-
-    // Set of properties, which are constant.
-    struct properties_t {
-        // Whether jobs in this group should have job control.
-        bool job_control{};
-
-        // Whether we should claim the terminal when we run in the foreground.
-        // TODO: this is effectively the same as job control, rationalize this.
-        bool wants_terminal{};
-
-        // Whether we are an internal job group.
-        bool is_internal{};
-
-        // The job ID of this group.
-        job_id_t job_id{};
-    };
-    const properties_t props_;
-
-    // The original command which produced this job tree.
-    const wcstring command_;
-
-    // Whether we are in the foreground, meaning that the user is waiting for this.
-    relaxed_atomic_bool_t is_foreground_{};
-
-    // Whether the root job is constructed. If not, we cannot reap it yet.
-    relaxed_atomic_bool_t root_constructed_{};
-
-    // If not zero, a signal indicating cancellation.
-    int cancel_signal_{};
-
-    job_group_t(const properties_t &props, wcstring command)
-        : props_(props), command_(std::move(command)) {}
-};
-
 /// A structure representing a single fish process. Contains variables for tracking process state
 /// and the process argument list. Actually, a fish process can be either a regular external
 /// process, an internal builtin which may or may not spawn a fake IO process during execution, a
@@ -390,15 +277,13 @@ class process_t {
 typedef std::unique_ptr<process_t> process_ptr_t;
 typedef std::vector<process_ptr_t> process_list_t;
 
+/// A user-visible job ID.
+using job_id_t = int;
+
 /// The non user-visible, never-recycled job ID.
 /// Every job has a unique positive value for this.
 using internal_job_id_t = uint64_t;
 
-/// The user-visible, optional, recycled job ID.
-using job_id_t = int;
-job_id_t acquire_job_id(void);
-void release_job_id(job_id_t jid);
-
 /// A struct representing a job. A job is a pipeline of one or more processes.
 class job_t {
    public:
@@ -491,11 +376,11 @@ class job_t {
     /// \return the pgid for the job, based on the job group.
     /// This may be none if the job consists of just internal fish functions or builtins.
     /// This may also be fish itself.
-    maybe_t<pid_t> get_pgid() const { return group->get_pgid(); }
+    maybe_t<pid_t> get_pgid() const;
 
     /// The id of this job.
     /// This is user-visible, is recycled, and may be -1.
-    job_id_t job_id() const { return group->get_id(); }
+    job_id_t job_id() const;
 
     /// A non-user-visible, never-recycled job ID.
     const internal_job_id_t internal_job_id;
@@ -560,7 +445,7 @@ class job_t {
     bool from_event_handler() const { return properties.from_event_handler; }
 
     /// \return whether this job's group is in the foreground.
-    bool is_foreground() const { return group->is_foreground(); }
+    bool is_foreground() const;
 
     /// \return whether we should report process exit events.
     /// This implements some historical behavior which has not been justified.