From b2ff4d6bc0bac3348d0e9de3e3c0694a108ab64e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 12 Aug 2023 16:26:07 -0700 Subject: [PATCH] Adopt Rust PosixSpawner This removes the C++ posix_spawner_t, adopting the Rust implementation. --- fish-rust/build.rs | 1 + fish-rust/src/spawn.rs | 69 ++++++++++++++++++++++- src/exec.cpp | 23 ++++++-- src/postfork.cpp | 122 ----------------------------------------- src/postfork.h | 31 ----------- 5 files changed, 86 insertions(+), 160 deletions(-) diff --git a/fish-rust/build.rs b/fish-rust/build.rs index 07b1d31d5..507db90e1 100644 --- a/fish-rust/build.rs +++ b/fish-rust/build.rs @@ -66,6 +66,7 @@ fn main() { "src/redirection.rs", "src/signal.rs", "src/smoke.rs", + "src/spawn.rs", "src/termsize.rs", "src/threads.rs", "src/timer.rs", diff --git a/fish-rust/src/spawn.rs b/fish-rust/src/spawn.rs index 0c64601a2..29d4c5bb0 100644 --- a/fish-rust/src/spawn.rs +++ b/fish-rust/src/spawn.rs @@ -3,7 +3,7 @@ use crate::ffi::{self, job_t}; use crate::redirection::Dup2List; use crate::signal::get_signals_with_handlers; -use errno::{self, Errno}; +use errno::{self, set_errno, Errno}; use libc::{self, c_char, posix_spawn_file_actions_t, posix_spawnattr_t}; use std::ffi::{CStr, CString}; @@ -158,9 +158,9 @@ pub fn new(j: &job_t, dup2s: &Dup2List) -> Result { Ok(PosixSpawner { attr, actions }) } - // Consume this spawner, attempting to spawn a new process. + // Attempt to spawn a new process. pub fn spawn( - self, + &mut self, cmd: *const c_char, argv: *const *mut c_char, envp: *const *mut c_char, @@ -226,3 +226,66 @@ fn get_path_bshell() -> CString { // which fail to run Thompson shell scripts; we simply assume it is /bin/sh. CString::new("/bin/sh").unwrap() } + +/// Returns a Box::into_raw(), or nullptr on error, in which case errno is set. +/// j is an unowned pointer to a job_t, dup2s is an unowned pointer to a Dup2List. +fn new_spawner_ffi(j: *const u8, dup2s: *const u8) -> *mut PosixSpawner { + let j: &job_t = unsafe { &*j.cast() }; + let dup2s: &Dup2List = unsafe { &*dup2s.cast() }; + match PosixSpawner::new(j, dup2s) { + Ok(spawner) => Box::into_raw(Box::new(spawner)), + Err(err) => { + set_errno(err); + std::ptr::null_mut() + } + } +} + +impl Drop for PosixSpawner { + fn drop(&mut self) { + // Necessary to define this for FFI purposes, to avoid link errors. + } +} + +impl PosixSpawner { + /// Returns a pid, or -1, in which case errno is set. + fn spawn_ffi( + &mut self, + cmd: *const c_char, + argv: *const *mut c_char, + envp: *const *mut c_char, + ) -> i32 { + match self.spawn(cmd, argv, envp) { + Ok(pid) => pid, + Err(err) => { + set_errno(err); + -1 + } + } + } +} + +fn force_cxx_to_generate_box_do_not_call() -> Box { + unimplemented!("Do not call, for linking help only"); +} + +#[cxx::bridge] +mod posix_spawn_ffi { + extern "Rust" { + type PosixSpawner; + + // Required to force cxx to generate a Box destructor, to avoid link errors. + fn force_cxx_to_generate_box_do_not_call() -> Box; + + #[cxx_name = "new_spawner"] + fn new_spawner_ffi(j: *const u8, dup2s: *const u8) -> *mut PosixSpawner; + + #[cxx_name = "spawn"] + fn spawn_ffi( + &mut self, + cmd: *const c_char, + argv: *const *mut c_char, + envp: *const *mut c_char, + ) -> i32; + } +} diff --git a/src/exec.cpp b/src/exec.cpp index 96fff3782..5eb926382 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -31,6 +31,7 @@ #include "ast.h" #include "builtin.h" #include "common.h" +#include "cxx.h" #include "env.h" #include "env_dispatch.rs.h" #include "exec.h" @@ -51,6 +52,7 @@ #include "proc.h" #include "reader.h" #include "redirection.h" +#include "spawn.rs.h" #include "timer.rs.h" #include "trace.rs.h" #include "wcstringutil.h" @@ -536,10 +538,23 @@ static launch_result_t exec_external_command(parser_t &parser, const std::shared if (can_use_posix_spawn_for_job(j, dup2s)) { ++s_fork_count; // spawn counts as a fork+exec - posix_spawner_t spawner(j.get(), dup2s); - maybe_t pid = spawner.spawn(actual_cmd, const_cast(argv), - const_cast(envv)); - if (int err = spawner.get_error()) { + int err = 0; + maybe_t pid = none(); + PosixSpawner *raw_spawner = + new_spawner(reinterpret_cast(j.get()), reinterpret_cast(&dup2s)); + if (raw_spawner == nullptr) { + err = errno; + } else { + auto spawner = rust::Box::from_raw(raw_spawner); + auto pid_or_neg = spawner->spawn(actual_cmd, const_cast(argv), + const_cast(envv)); + if (pid_or_neg > 0) { + pid = pid_or_neg; + } else { + err = errno; + } + } + if (err) { safe_report_exec_error(err, actual_cmd, argv, envv); p->status = proc_status_t::from_exit_code(exit_code_from_exec_error(err)); return launch_result_t::failed; diff --git a/src/postfork.cpp b/src/postfork.cpp index d7141f6fa..ecbfac160 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -246,128 +246,6 @@ pid_t execute_fork() { return 0; } -#if FISH_USE_POSIX_SPAWN - -// Given an error code, if it is the first error, record it. -// \return whether we have any error. -bool posix_spawner_t::check_fail(int err) { - if (error_ == 0) error_ = err; - return error_ != 0; -} - -posix_spawner_t::~posix_spawner_t() { - if (attr_.has_value()) { - posix_spawnattr_destroy(this->attr()); - } - if (actions_.has_value()) { - posix_spawn_file_actions_destroy(this->actions()); - } -} - -posix_spawner_t::posix_spawner_t(const job_t *j, const dup2_list_t &dup2s) { - // Initialize our fields. This may fail. - { - posix_spawnattr_t attr; - if (check_fail(posix_spawnattr_init(&attr))) return; - this->attr_ = attr; - } - - { - posix_spawn_file_actions_t actions; - if (check_fail(posix_spawn_file_actions_init(&actions))) return; - this->actions_ = actions; - } - - // desired_pgid tracks the pgroup for the process. If it is none, the pgroup is left unchanged. - // If it is zero, create a new pgroup from the pid. If it is >0, join that pgroup. - maybe_t desired_pgid = none(); - { - auto pgid = j->group->get_pgid(); - if (pgid) { - desired_pgid = pgid->value; - } else if (j->processes.front()->leads_pgrp) { - desired_pgid = 0; - } - } - - // Set the handling for job control signals back to the default. - bool reset_signal_handlers = true; - - // Remove all signal blocks. - bool reset_sigmask = true; - - // Set our flags. - short flags = 0; - if (reset_signal_handlers) flags |= POSIX_SPAWN_SETSIGDEF; - if (reset_sigmask) flags |= POSIX_SPAWN_SETSIGMASK; - if (desired_pgid.has_value()) flags |= POSIX_SPAWN_SETPGROUP; - - if (check_fail(posix_spawnattr_setflags(attr(), flags))) return; - - if (desired_pgid.has_value()) { - if (check_fail(posix_spawnattr_setpgroup(attr(), *desired_pgid))) return; - } - - // Everybody gets default handlers. - if (reset_signal_handlers) { - sigset_t sigdefault; - get_signals_with_handlers(&sigdefault); - if (check_fail(posix_spawnattr_setsigdefault(attr(), &sigdefault))) return; - } - - // No signals blocked. - if (reset_sigmask) { - sigset_t sigmask; - sigemptyset(&sigmask); - blocked_signals_for_job(*j, &sigmask); - if (check_fail(posix_spawnattr_setsigmask(attr(), &sigmask))) return; - } - - // Apply our dup2s. - for (const auto &act : dup2s.get_actions()) { - if (act.target < 0) { - if (check_fail(posix_spawn_file_actions_addclose(actions(), act.src))) return; - } else { - if (check_fail(posix_spawn_file_actions_adddup2(actions(), act.src, act.target))) - return; - } - } -} - -maybe_t posix_spawner_t::spawn(const char *cmd, char *const argv[], char *const envp[]) { - if (get_error()) return none(); - pid_t pid = -1; - if (check_fail(posix_spawn(&pid, cmd, &*actions_, &*attr_, argv, envp))) { - // The shebang wasn't introduced until UNIX Seventh Edition, so if - // the kernel won't run the binary we hand it off to the interpreter - // after performing a binary safety check, recommended by POSIX: a - // line needs to exist before the first \0 with a lowercase letter - if (error_ == ENOEXEC && is_thompson_shell_script(cmd)) { - error_ = 0; - // Create a new argv with /bin/sh prepended. - std::vector argv2; - char interp[] = _PATH_BSHELL; - argv2.push_back(interp); - // The command to call should use the full path, - // not what we would pass as argv0. - std::string cmd2 = cmd; - argv2.push_back(&cmd2[0]); - for (size_t i = 1; argv[i] != nullptr; i++) { - argv2.push_back(argv[i]); - } - argv2.push_back(nullptr); - if (check_fail(posix_spawn(&pid, interp, &*actions_, &*attr_, &argv2[0], envp))) { - return none(); - } - } else { - return none(); - } - } - return pid; -} - -#endif // FISH_USE_POSIX_SPAWN - void safe_report_exec_error(int err, const char *actual_cmd, const char *const *argv, const char *const *envv) { switch (err) { diff --git a/src/postfork.h b/src/postfork.h index cc2eb59c5..681cd3b70 100644 --- a/src/postfork.h +++ b/src/postfork.h @@ -49,35 +49,4 @@ pid_t execute_fork(); void safe_report_exec_error(int err, const char *actual_cmd, const char *const *argv, const char *const *envv); -#if FISH_USE_POSIX_SPAWN -/// A RAII type which wraps up posix_spawn's data structures. -class posix_spawner_t : noncopyable_t, nonmovable_t { - public: - /// Attempt to construct from a job and dup2 list. - /// The caller must check the error function, as this may fail. - posix_spawner_t(const job_t *j, const dup2_list_t &dup2s); - - /// \return the last error code, or 0 if there is no error. - int get_error() const { return error_; } - - /// If this spawner does not have an error, invoke posix_spawn. Parameters are the same as - /// posix_spawn. - /// \return the pid, or none() on failure, in which case our error will be set. - maybe_t spawn(const char *cmd, char *const argv[], char *const envp[]); - - ~posix_spawner_t(); - - private: - bool check_fail(int err); - posix_spawnattr_t *attr() { return &*attr_; } - posix_spawn_file_actions_t *actions() { return &*actions_; } - - posix_spawner_t(); - int error_{0}; - maybe_t attr_{}; - maybe_t actions_{}; -}; - -#endif - #endif