Port job_group to rust (#9608)

More ugliness with types that cxx bridge can't recognize as being POD. Using
pointers to get/set `termios` values with an assert to make sure we're using
identical definitions on both sides (in cpp from the system headers and in rust
from the libc crate as exported).

I don't know why cxx bridge doesn't allow `SharedPtr<OpaqueRustType>` but we can
work around it in C++ by converting a `Box<T>` to a `shared_ptr<T>` then convert
it back when it needs to be destructed. I can't find a clean way of doing it
from the cxx bridge wrapper so for now it needs to be done manually in the C++
code.

Types/values that are drop-in ready over ffi are renamed to match the old cpp
names but for types that now differ due to ffi difficulties I've left the `_ffi`
in the function names to indicate that this isn't the "correct" way of using the
types/methods.

CMakeLists.txt

@@ -120,7 +120,7 @@ set(FISH_SRCS
     src/exec.cpp src/expand.cpp src/fallback.cpp src/fish_version.cpp
     src/flog.cpp src/function.cpp src/highlight.cpp
     src/history.cpp src/history_file.cpp src/input.cpp src/input_common.cpp
-    src/io.cpp src/iothread.cpp src/job_group.cpp src/kill.cpp
+    src/io.cpp src/iothread.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

fish-rust/build.rs

@@ -26,6 +26,7 @@ fn main() -> miette::Result<()> {
         "src/ffi_init.rs",
         "src/ffi_tests.rs",
         "src/future_feature_flags.rs",
+        "src/job_group.rs",
         "src/parse_constants.rs",
         "src/redirection.rs",
         "src/smoke.rs",

fish-rust/src/common.rs

@@ -111,3 +111,11 @@ pub fn valid_func_name(name: &wstr) -> bool {
     };
     true
 }
+
+pub const fn assert_send<T: Send>() -> () {
+    ()
+}
+
+pub const fn assert_sync<T: Sync>() -> () {
+    ()
+}

fish-rust/src/job_group.rs

@@ -0,0 +1,352 @@
+use self::job_group::pgid_t;
+use crate::common::{assert_send, assert_sync};
+use crate::wchar_ffi::{WCharFromFFI, WCharToFFI};
+use cxx::{CxxWString, UniquePtr};
+use std::num::{NonZeroI32, NonZeroU32};
+use std::sync::atomic::{AtomicBool, AtomicI32, Ordering};
+use std::sync::Mutex;
+use widestring::WideUtfString;
+
+#[cxx::bridge]
+mod job_group {
+    // Not only does cxx bridge not recognize libc::pid_t, it doesn't even recognize i32 as a POD
+    // type! :sadface:
+    struct pgid_t {
+        value: i32,
+    }
+
+    extern "Rust" {
+        #[cxx_name = "job_group_t"]
+        type JobGroup;
+
+        fn wants_job_control(&self) -> bool;
+        fn wants_terminal(&self) -> bool;
+        fn is_foreground(&self) -> bool;
+        fn set_is_foreground(&self, value: bool);
+        #[cxx_name = "get_command"]
+        fn get_command_ffi(&self) -> UniquePtr<CxxWString>;
+        #[cxx_name = "get_job_id"]
+        fn get_job_id_ffi(&self) -> i32;
+        #[cxx_name = "get_cancel_signal"]
+        fn get_cancel_signal_ffi(&self) -> i32;
+        #[cxx_name = "cancel_with_signal"]
+        fn cancel_with_signal_ffi(&self, signal: i32);
+        fn set_pgid(&mut self, pgid: i32);
+        #[cxx_name = "get_pgid"]
+        fn get_pgid_ffi(&self) -> UniquePtr<pgid_t>;
+        fn has_job_id(&self) -> bool;
+
+        // cxx bridge doesn't recognize `libc::*` as being POD types, so it won't let us use them in
+        // a SharedPtr/UniquePtr/Box and won't let us pass/return them by value/reference, either.
+        unsafe fn get_modes_ffi(&self, size: usize) -> *const u8; /* actually `* const libc::termios` */
+        unsafe fn set_modes_ffi(&mut self, modes: *const u8, size: usize); /* actually `* const libc::termios` */
+
+        // The C++ code uses `shared_ptr<JobGroup>` but cxx bridge doesn't support returning a
+        // `SharedPtr<OpaqueRustType>` nor does it implement `Arc<T>` so we return a box and then
+        // convert `rust::box<T>` to `std::shared_ptr<T>` with `box_to_shared_ptr()` (from ffi.h).
+        fn create_job_group_ffi(command: &CxxWString, wants_job_id: bool) -> Box<JobGroup>;
+        fn create_job_group_with_job_control_ffi(
+            command: &CxxWString,
+            wants_term: bool,
+        ) -> Box<JobGroup>;
+    }
+}
+
+fn create_job_group_ffi(command: &CxxWString, wants_job_id: bool) -> Box<JobGroup> {
+    let job_group = JobGroup::create(command.from_ffi(), wants_job_id);
+    Box::new(job_group)
+}
+
+fn create_job_group_with_job_control_ffi(command: &CxxWString, wants_term: bool) -> Box<JobGroup> {
+    let job_group = JobGroup::create_with_job_control(command.from_ffi(), wants_term);
+    Box::new(job_group)
+}
+
+/// A job id, corresponding to what is printed by `jobs`. 1 is the first valid job id.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
+#[repr(transparent)]
+pub struct JobId(NonZeroU32);
+
+/// `JobGroup` 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 `JobGroup` is intended to eventually be
+/// shared between threads, and so must be thread safe.
+#[derive(Debug)]
+pub struct JobGroup {
+    /// 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 when resuming a stopped job.
+    pub tmodes: Option<libc::termios>,
+    /// Whether job control is enabled in this `JobGroup` or not.
+    ///
+    /// If this is set, then the first process in the root job must be external, as it will become
+    /// the process group leader.
+    pub job_control: bool,
+    /// Whether we should `tcsetpgrp()` the job when it runs in the foreground. Should be checked
+    /// via [`Self::wants_terminal()`] only.
+    wants_term: bool,
+    /// Whether we are in the foreground, meaning the user is waiting for this job to complete.
+    pub is_foreground: AtomicBool,
+    /// The pgid leading our group. This is only ever set if [`job_control`](Self::JobControl) is
+    /// true. We ensure the value (when set) is always non-negative.
+    pgid: Option<libc::pid_t>,
+    /// The original command which produced this job tree.
+    pub command: WideUtfString,
+    /// Our job id, if any. `None` here should evaluate to `-1` for ffi purposes.
+    /// "Simple block" groups like function calls do not have a job id.
+    pub job_id: Option<JobId>,
+    /// The signal causing the group to cancel or `0` if none.
+    /// Not using an `Option<NonZeroI32>` to be able to atomically load/store to this field.
+    signal: AtomicI32,
+}
+
+const _: () = assert_send::<JobGroup>();
+const _: () = assert_sync::<JobGroup>();
+
+impl JobGroup {
+    /// Whether this job wants job control.
+    pub fn wants_job_control(&self) -> bool {
+        self.job_control
+    }
+
+    /// If this job should own the terminal when it runs. True only if both [`Self::wants_term]` and
+    /// [`Self::is_foreground`] are true.
+    pub fn wants_terminal(&self) -> bool {
+        self.wants_term && self.is_foreground()
+    }
+
+    /// Whether we are the currently the foreground group. Should never be true for more than one
+    /// `JobGroup` at any given moment.
+    pub fn is_foreground(&self) -> bool {
+        self.is_foreground.load(Ordering::Relaxed)
+    }
+
+    /// Mark whether we are in the foreground.
+    pub fn set_is_foreground(&self, in_foreground: bool) {
+        self.is_foreground.store(in_foreground, Ordering::Relaxed);
+    }
+
+    /// Return the command which produced this job tree.
+    pub fn get_command_ffi(&self) -> UniquePtr<CxxWString> {
+        self.command.to_ffi()
+    }
+
+    /// Return the job id or -1 if none.
+    pub fn get_job_id_ffi(&self) -> i32 {
+        self.job_id.map(|j| u32::from(j.0) as i32).unwrap_or(-1)
+    }
+
+    /// Returns whether we have valid job id. "Simple block" groups like function calls do not.
+    pub fn has_job_id(&self) -> bool {
+        self.job_id.is_some()
+    }
+
+    /// Gets the cancellation signal, if any.
+    pub fn get_cancel_signal(&self) -> Option<NonZeroI32> {
+        match self.signal.load(Ordering::Relaxed) {
+            0 => None,
+            s => Some(NonZeroI32::new(s).unwrap()),
+        }
+    }
+
+    /// Gets the cancellation signal or `0` if none.
+    pub fn get_cancel_signal_ffi(&self) -> i32 {
+        // Legacy C++ code expects a zero in case of no signal.
+        self.get_cancel_signal().map(|s| s.into()).unwrap_or(0)
+    }
+
+    /// Mark that a process in this group got a signal and should cancel.
+    pub fn cancel_with_signal(&self, signal: NonZeroI32) {
+        // We only assign the signal if one hasn't yet been assigned. This means the first signal to
+        // register wins over any that come later.
+        self.signal
+            .compare_exchange(0, signal.into(), Ordering::Relaxed, Ordering::Relaxed)
+            .ok();
+    }
+
+    /// Mark that a process in this group got a signal and should cancel
+    pub fn cancel_with_signal_ffi(&self, signal: i32) {
+        self.cancel_with_signal(signal.try_into().expect("Invalid zero signal!"));
+    }
+
+    /// Set the pgid for this job group, latching it to this value. This should only be called if
+    /// job control is active for this group. The pgid should not already have been set, and should
+    /// be different from fish's pgid. Of course this does not keep the pgid alive by itself.
+    ///
+    /// Note we need not be concerned about thread safety. job_groups are intended to be shared
+    /// across threads, but any pgid should always have been set beforehand, since it's set
+    /// immediately after the first process launches.
+    ///
+    /// As such, this method takes `&mut self` rather than `&self` to enforce that this operation is
+    /// only available during initial construction/initialization.
+    pub fn set_pgid(&mut self, pgid: libc::pid_t) {
+        assert!(
+            self.wants_job_control(),
+            "Should not set a pgid for a group that doesn't want job control!"
+        );
+        assert!(pgid >= 0, "Invalid pgid!");
+        assert!(self.pgid.is_none(), "JobGroup::pgid already set!");
+
+        self.pgid = Some(pgid);
+    }
+
+    /// Returns the value of [`JobGroup::pgid`]. This is never fish's own pgid!
+    pub fn get_pgid(&self) -> Option<libc::pid_t> {
+        self.pgid
+    }
+
+    /// Returns the value of [`JobGroup::pgid`] in a `UniquePtr<T>` to take the place of an
+    /// `Option<T>` for ffi purposes. A null `UniquePtr` is equivalent to `None`.
+    pub fn get_pgid_ffi(&self) -> cxx::UniquePtr<pgid_t> {
+        match self.pgid {
+            Some(value) => UniquePtr::new(pgid_t { value }),
+            None => UniquePtr::null(),
+        }
+    }
+
+    /// Returns the current terminal modes associated with the `JobGroup` for ffi purposes.
+    unsafe fn get_modes_ffi(&self, size: usize) -> *const u8 {
+        assert_eq!(
+            size,
+            core::mem::size_of::<libc::termios>(),
+            "Mismatch between expected and actual ffi size of struct termios!"
+        );
+
+        self.tmodes
+            .as_ref()
+            // Really cool that type inference works twice in a row here. The first `_` is deduced
+            // from the left and the second `_` is deduced from the right (the return type).
+            .map(|val| val as *const _ as *const _)
+            .unwrap_or(core::ptr::null())
+    }
+
+    /// Sets the current terminal modes associated with the `JobGroup`. Only use for ffi.
+    ///
+    /// Unlike `set_pgid()`, this isn't documented in the C++ codebase as being only called at
+    /// initialization but as the underlying [`self.tmodes`] wasn't wrapped in any sort of
+    /// thread-safe marshalling struct, we'll assume it can only be called from one thread and use
+    /// `&mut self` for safety.
+    unsafe fn set_modes_ffi(&mut self, modes: *const u8, size: usize) {
+        assert_eq!(
+            size,
+            core::mem::size_of::<libc::termios>(),
+            "Mismatch between expected and actual ffi size of struct termios!"
+        );
+
+        let modes = modes as *const libc::termios;
+        if modes.is_null() {
+            self.tmodes = None;
+        } else {
+            self.tmodes = Some(*modes);
+        }
+    }
+}
+
+/// Basic thread-safe sorted vector of job ids currently in use.
+///
+/// In the C++ codebase, this is deliberately leaked to avoid destructor ordering issues - see
+/// #6539. Rust automatically "leaks" all `static` variables (does not call their `Drop` impls)
+/// because of the inherent difficulty in doing that correctly (i.e. what we ran into).
+static CONSUMED_JOB_IDS: Mutex<Vec<JobId>> = Mutex::new(Vec::new());
+
+impl JobId {
+    const NONE: Option<JobId> = None;
+
+    /// Return a `JobId` that is greater than all extant job ids stored in [`CONSUMED_JOB_IDS`].
+    /// The `JobId` should be freed with [`JobId::release()`] when it is no longer in use.
+    fn acquire() -> Option<Self> {
+        let mut consumed_job_ids = CONSUMED_JOB_IDS.lock().expect("Poisoned mutex!");
+
+        // The new job id should be greater than the largest currently used id (#6053). The job ids
+        // in CONSUMED_JOB_IDS are sorted in ascending order, so we just have to check the last.
+        let job_id = consumed_job_ids
+            .last()
+            .map(JobId::next)
+            .unwrap_or(JobId(1.try_into().unwrap()));
+        consumed_job_ids.push(job_id);
+        return Some(job_id);
+    }
+
+    /// Remove the provided `JobId` from [`CONSUMED_JOB_IDS`].
+    fn release(id: JobId) {
+        let mut consumed_job_ids = CONSUMED_JOB_IDS.lock().expect("Poisoned mutex!");
+
+        let pos = consumed_job_ids
+            .binary_search(&id)
+            .expect("Job id was not in use!");
+        consumed_job_ids.remove(pos);
+    }
+
+    /// Increments the internal id and returns it wrapped in a new `JobId`.
+    fn next(&self) -> JobId {
+        JobId(self.0.checked_add(1).expect("Job id overflow!"))
+    }
+}
+
+impl JobGroup {
+    pub fn new(
+        command: WideUtfString,
+        id: Option<JobId>,
+        job_control: bool,
+        wants_term: bool,
+    ) -> Self {
+        // We *can* have a job id without job control, but not the reverse.
+        if job_control {
+            assert!(id.is_some(), "Cannot have job control without a job id!");
+        }
+        if wants_term {
+            assert!(job_control, "Cannot take terminal without job control!");
+        }
+
+        Self {
+            job_id: id,
+            job_control,
+            wants_term,
+            command,
+            tmodes: None,
+            signal: 0.into(),
+            is_foreground: false.into(),
+            pgid: None,
+        }
+    }
+
+    /// Return a new `JobGroup` with the provided `command`. The `JobGroup` is only assigned a
+    /// `JobId` if `wants_job_id` is true and is created with job control disabled and
+    /// [`JobGroup::wants_term`] set to false.
+    pub fn create(command: WideUtfString, wants_job_id: bool) -> JobGroup {
+        JobGroup::new(
+            command,
+            if wants_job_id {
+                JobId::acquire()
+            } else {
+                JobId::NONE
+            },
+            false, /* job_control */
+            false, /* wants_term */
+        )
+    }
+
+    /// Return a new `JobGroup` with the provided `command` with job control enabled. A [`JobId`] is
+    /// automatically acquired and assigned. If `wants_term` is true then [`JobGroup::wants_term`]
+    /// is also set to `true` accordingly.
+    pub fn create_with_job_control(command: WideUtfString, wants_term: bool) -> JobGroup {
+        JobGroup::new(
+            command,
+            JobId::acquire(),
+            true, /* job_control */
+            wants_term,
+        )
+    }
+}
+
+impl Drop for JobGroup {
+    fn drop(&mut self) {
+        if let Some(job_id) = self.job_id {
+            JobId::release(job_id);
+        }
+    }
+}

fish-rust/src/lib.rs

@@ -18,6 +18,7 @@ mod ffi_init;
 mod ffi_tests;
 mod flog;
 mod future_feature_flags;
+mod job_group;
 mod nix;
 mod parse_constants;
 mod redirection;

fish-rust/src/signal.rs

@@ -1,8 +1,7 @@
-use widestring::U32CStr;
-
 use crate::ffi;
 use crate::topic_monitor::{generation_t, invalid_generations, topic_monitor_principal, topic_t};
 use crate::wchar_ffi::{c_str, wstr};
+use widestring::U32CStr;
 
 /// A sigint_detector_t can be used to check if a SIGINT (or SIGHUP) has been delivered.
 pub struct sigchecker_t {

src/builtins/bg.cpp

@@ -14,11 +14,11 @@
 #include "../common.h"
 #include "../fallback.h"  // IWYU pragma: keep
 #include "../io.h"
-#include "../job_group.h"
 #include "../maybe.h"
 #include "../parser.h"
 #include "../proc.h"
 #include "../wutil.h"  // IWYU pragma: keep
+#include "job_group.rs.h"
 
 /// Helper function for builtin_bg().
 static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) {

src/builtins/fg.cpp

@@ -20,13 +20,13 @@
 #include "../fallback.h"  // IWYU pragma: keep
 #include "../fds.h"
 #include "../io.h"
-#include "../job_group.h"
 #include "../maybe.h"
 #include "../parser.h"
 #include "../proc.h"
 #include "../reader.h"
 #include "../tokenizer.h"
 #include "../wutil.h"  // IWYU pragma: keep
+#include "job_group.rs.h"
 
 /// Builtin for putting a job in the foreground.
 maybe_t<int> builtin_fg(parser_t &parser, io_streams_t &streams, const wchar_t **argv) {
@@ -122,8 +122,9 @@ maybe_t<int> builtin_fg(parser_t &parser, io_streams_t &streams, const wchar_t *
     parser.job_promote(job);
     make_fd_blocking(STDIN_FILENO);
     job->group->set_is_foreground(true);
-    if (job->group->wants_terminal() && job->group->tmodes) {
-        int res = tcsetattr(STDIN_FILENO, TCSADRAIN, &job->group->tmodes.value());
+    if (job->group->wants_terminal() && (job->group->get_modes_ffi(sizeof(termios)) != nullptr)) {
+        auto *termios = (struct termios *)job->group->get_modes_ffi(sizeof(struct termios));
+        int res = tcsetattr(STDIN_FILENO, TCSADRAIN, termios);
         if (res < 0) wperror(L"tcsetattr");
     }
     tty_transfer_t transfer;

src/exec.cpp

@@ -38,7 +38,7 @@
 #include "global_safety.h"
 #include "io.h"
 #include "iothread.h"
-#include "job_group.h"
+#include "job_group.rs.h"
 #include "maybe.h"
 #include "null_terminated_array.h"
 #include "parse_tree.h"
@@ -417,9 +417,9 @@ static launch_result_t fork_child_for_process(const std::shared_ptr<job_t> &job,
     }
     {
         auto pgid = job->group->get_pgid();
-        if (pgid.has_value()) {
-            if (int err = execute_setpgid(p->pid, *pgid, is_parent)) {
-                report_setpgid_error(err, is_parent, *pgid, job.get(), p);
+        if (pgid) {
+            if (int err = execute_setpgid(p->pid, pgid->value, is_parent)) {
+                report_setpgid_error(err, is_parent, pgid->value, job.get(), p);
             }
         }
     }

src/ffi.h

@@ -0,0 +1,15 @@
+#include <algorithm>
+#include <memory>
+
+#include "cxx.h"
+#if INCLUDE_RUST_HEADERS
+// For some unknown reason, the definition of rust::Box is in this particular header:
+#include "parse_constants.rs.h"
+#endif
+
+template <typename T>
+std::shared_ptr<T> box_to_shared_ptr(rust::Box<T> &&value) {
+    T *ptr = value.into_raw();
+    std::shared_ptr<T> shared(ptr, [](T *ptr) { rust::Box<T>::from_raw(ptr); });
+    return shared;
+}

src/io.h

@@ -21,7 +21,7 @@
 
 using std::shared_ptr;
 
-class job_group_t;
+struct job_group_t;
 
 /// separated_buffer_t represents a buffer of output from commands, prepared to be turned into a
 /// variable. For example, command substitutions output into one of these. Most commands just

src/job_group.cpp

@@ -1,69 +0,0 @@
-#include "config.h"  // IWYU pragma: keep
-
-#include "job_group.h"
-
-#include <algorithm>
-#include <utility>
-#include <vector>
-
-#include "common.h"
-#include "fallback.h"  // IWYU pragma: keep
-#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(wcstring command, job_id_t job_id, bool job_control, bool wants_terminal)
-    : job_control_(job_control),
-      wants_terminal_(wants_terminal),
-      command_(std::move(command)),
-      job_id_(job_id) {}
-
-job_group_t::~job_group_t() {
-    if (job_id_ > 0) {
-        release_job_id(job_id_);
-    }
-}
-
-// static
-job_group_ref_t job_group_t::create(wcstring command, bool wants_job_id) {
-    job_id_t jid = wants_job_id ? acquire_job_id() : 0;
-    return job_group_ref_t(new job_group_t(std::move(command), jid));
-}
-
-// static
-job_group_ref_t job_group_t::create_with_job_control(wcstring command, bool wants_terminal) {
-    return job_group_ref_t(new job_group_t(std::move(command), acquire_job_id(),
-                                           true /* job_control */, wants_terminal));
-}
-
-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 any pgid should always have been set beforehand, since it's set
-    // immediately after the first process launches.
-    assert(pgid >= 0 && "invalid pgid");
-    assert(wants_job_control() && "should not set a pgid for this group");
-    assert(!pgid_.has_value() && "pgid already set");
-    pgid_ = pgid;
-}

src/job_group.h

@@ -1,111 +0,0 @@
-#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"
-#include "maybe.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_group_t;
-using job_group_ref_t = std::shared_ptr<job_group_t>;
-
-class job_group_t {
-   public:
-    /// \return whether this group wants job control.
-    bool wants_job_control() const { return job_control_; }
-
-    /// \return if this job group should own the terminal when it runs.
-    bool wants_terminal() const { return wants_terminal_ && is_foreground(); }
-
-    /// \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 the command which produced this job tree.
-    const wcstring &get_command() const { return command_; }
-
-    /// \return the job ID, or -1 if none.
-    job_id_t get_job_id() const { return job_id_; }
-
-    /// \return whether we have a valid job ID. "Simple block" groups like function calls do not.
-    bool has_job_id() const { return job_id_ > 0; }
-
-    /// Get the cancel signal, or 0 if none.
-    int get_cancel_signal() const { return signal_; }
-
-    /// Mark that a process in this group got a signal, and so should cancel.
-    void cancel_with_signal(int signal) {
-        assert(signal > 0 && "Invalid cancel signal");
-        signal_.compare_exchange(0, signal);
-    }
-
-    /// 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 when resuming a stopped job.
-    maybe_t<struct termios> tmodes{};
-
-    /// Set the pgid for this job group, latching it to this value.
-    /// This should only be called if job control is active for this group.
-    /// The pgid should not already have been set, and should be different from fish's pgid.
-    /// Of course this does not keep the pgid alive by itself.
-    void set_pgid(pid_t pgid);
-
-    /// Get the pgid. This never returns fish's pgid.
-    maybe_t<pid_t> get_pgid() const { return pgid_; }
-
-    /// Construct a group for a job that will live internal to fish, optionally claiming a job ID.
-    static job_group_ref_t create(wcstring command, bool wants_job_id);
-
-    /// Construct a group for a job which will assign its first process as pgroup leader.
-    static job_group_ref_t create_with_job_control(wcstring command, bool wants_terminal);
-
-    ~job_group_t();
-
-   private:
-    job_group_t(wcstring command, job_id_t job_id, bool job_control = false,
-                bool wants_terminal = false);
-
-    // Whether job control is enabled.
-    // If this is set, then the first process in the root job must be external.
-    // It will become the process group leader.
-    const bool job_control_;
-
-    // Whether we should tcsetpgrp to the job when it runs in the foreground.
-    const bool wants_terminal_;
-
-    // Whether we are in the foreground, meaning that the user is waiting for this.
-    relaxed_atomic_bool_t is_foreground_{};
-
-    // The pgid leading our group. This is only ever set if job_control_ is true.
-    // This is never fish's pgid.
-    maybe_t<pid_t> pgid_{};
-
-    // The original command which produced this job tree.
-    const wcstring command_;
-
-    /// Our job ID. -1 if none.
-    const job_id_t job_id_;
-
-    /// The signal causing us the group to cancel, or 0.
-    relaxed_atomic_t<int> signal_{0};
-};
-
-#endif

src/operation_context.h

@@ -9,7 +9,7 @@
 
 class environment_t;
 class parser_t;
-class job_group_t;
+struct job_group_t;
 
 /// A common helper which always returns false.
 bool no_cancel();

src/parse_execution.cpp

@@ -27,10 +27,11 @@
 #include "event.h"
 #include "exec.h"
 #include "expand.h"
+#include "ffi.h"
 #include "flog.h"
 #include "function.h"
 #include "io.h"
-#include "job_group.h"
+#include "job_group.rs.h"
 #include "maybe.h"
 #include "operation_context.h"
 #include "parse_constants.h"
@@ -1557,12 +1558,14 @@ void parse_execution_context_t::setup_group(job_t *j) {
 
     if (j->processes.front()->is_internal() || !this->use_job_control()) {
         // This job either doesn't have a pgroup (e.g. a simple block), or lives in fish's pgroup.
-        j->group = job_group_t::create(j->command(), j->wants_job_id());
+        rust::Box<job_group_t> group = create_job_group_ffi(j->command(), j->wants_job_id());
+        j->group = box_to_shared_ptr(std::move(group));
     } else {
         // This is a "real job" that gets its own pgroup.
         j->processes.front()->leads_pgrp = true;
         bool wants_terminal = !parser->libdata().is_event;
-        j->group = job_group_t::create_with_job_control(j->command(), wants_terminal);
+        auto group = create_job_group_with_job_control_ffi(j->command(), wants_terminal);
+        j->group = box_to_shared_ptr(std::move(group));
     }
     j->group->set_is_foreground(!j->is_initially_background());
     j->mut_flags().is_group_root = true;

src/parser.cpp

@@ -25,7 +25,7 @@
 #include "fds.h"
 #include "flog.h"
 #include "function.h"
-#include "job_group.h"
+#include "job_group.rs.h"
 #include "parse_constants.h"
 #include "parse_execution.h"
 #include "proc.h"

src/parser.h

@@ -16,7 +16,6 @@
 #include "cxx.h"
 #include "env.h"
 #include "expand.h"
-#include "job_group.h"
 #include "maybe.h"
 #include "operation_context.h"
 #include "parse_constants.h"
@@ -32,6 +31,7 @@ class autoclose_fd_t;
 /// event_blockage_t represents a block on events.
 struct event_blockage_t {};
 
+struct job_group_t;
 typedef std::list<event_blockage_t> event_blockage_list_t;
 
 inline bool event_block_list_blocks_type(const event_blockage_list_t &ebls) {

src/postfork.cpp

@@ -20,7 +20,7 @@
 #include "fds.h"
 #include "flog.h"
 #include "iothread.h"
-#include "job_group.h"
+#include "job_group.rs.h"
 #include "postfork.h"
 #include "proc.h"
 #include "redirection.h"
@@ -283,8 +283,8 @@ posix_spawner_t::posix_spawner_t(const job_t *j, const dup2_list_t &dup2s) {
     maybe_t<pid_t> desired_pgid = none();
     {
         auto pgid = j->group->get_pgid();
-        if (pgid.has_value()) {
-            desired_pgid = *pgid;
+        if (pgid) {
+            desired_pgid = pgid->value;
         } else if (j->processes.front()->leads_pgrp) {
             desired_pgid = 0;
         }

src/proc.cpp

@@ -41,7 +41,7 @@
 #include "flog.h"
 #include "global_safety.h"
 #include "io.h"
-#include "job_group.h"
+#include "job_group.rs.h"
 #include "parser.h"
 #include "proc.h"
 #include "reader.h"
@@ -124,10 +124,10 @@ bool job_t::posts_job_exit_events() const {
 
 bool job_t::signal(int signal) {
     auto pgid = group->get_pgid();
-    if (pgid.has_value()) {
-        if (killpg(*pgid, signal) == -1) {
+    if (pgid) {
+        if (killpg(pgid->value, signal) == -1) {
             char buffer[512];
-            snprintf(buffer, 512, "killpg(%d, %s)", *pgid, strsignal(signal));
+            snprintf(buffer, 512, "killpg(%d, %s)", pgid->value, strsignal(signal));
             wperror(str2wcstring(buffer).c_str());
             return false;
         }
@@ -805,7 +805,7 @@ bool tty_transfer_t::try_transfer(const job_group_ref_t &jg) {
     }
 
     // Get the pgid; we must have one if we want the terminal.
-    pid_t pgid = *jg->get_pgid();
+    pid_t pgid = jg->get_pgid()->value;
     assert(pgid >= 0 && "Invalid pgid");
 
     // It should never be fish's pgroup.
@@ -904,7 +904,7 @@ bool tty_transfer_t::try_transfer(const job_group_ref_t &jg) {
             return false;
         } else {
             FLOGF(warning, _(L"Could not send job %d ('%ls') with pgid %d to foreground"),
-                  jg->get_job_id(), jg->get_command().c_str(), pgid);
+                  jg->get_job_id(), jg->get_command()->c_str(), pgid);
             wperror(L"tcsetpgrp");
             return false;
         }
@@ -926,7 +926,13 @@ bool tty_transfer_t::try_transfer(const job_group_ref_t &jg) {
 
 bool job_t::is_foreground() const { return group->is_foreground(); }
 
-maybe_t<pid_t> job_t::get_pgid() const { return group->get_pgid(); }
+maybe_t<pid_t> job_t::get_pgid() const {
+    auto pgid = group->get_pgid();
+    if (!pgid) {
+        return none();
+    }
+    return maybe_t<pid_t>{pgid->value};
+}
 
 maybe_t<pid_t> job_t::get_last_pid() const {
     for (auto iter = processes.rbegin(); iter != processes.rend(); ++iter) {
@@ -1004,7 +1010,7 @@ void tty_transfer_t::save_tty_modes() {
     if (owner_) {
         struct termios tmodes {};
         if (tcgetattr(STDIN_FILENO, &tmodes) == 0) {
-            owner_->tmodes = tmodes;
+            owner_->set_modes_ffi((uint8_t *)&tmodes, sizeof(struct termios));
         } else if (errno != ENOTTY) {
             wperror(L"tcgetattr");
         }

src/proc.h

@@ -18,7 +18,6 @@
 #include <vector>
 
 #include "common.h"
-#include "job_group.h"
 #include "maybe.h"
 #include "parse_tree.h"
 #include "redirection.h"
@@ -58,6 +57,7 @@ namespace ast {
 struct statement_t;
 }
 
+struct 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,