github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2024-11-23 20:54:04 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Clean up posix_spawn code paths

Browse Source 
Prior to this change, the posix_spawn code paths used a fair amount of
manual management around its allocated structures (attrs and file actions).
Encapsulate this into a new class that manages memory management and error
handling.
This commit is contained in:
ridiculousfish 2020-06-09 14:56:03 -07:00
parent 1b90be57f2
commit b4351c5927
3 changed files with 97 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/exec.cpp
View File

@ -518,44 +518,26 @@ static bool exec_external_command(parser_t &parser, const std::shared_ptr<job_t>
bool use_posix_spawn = g_use_posix_spawn && can_use_posix_spawn_for_job(j, dup2s);
if (use_posix_spawn) {
s_fork_count++; // spawn counts as a fork+exec
// Create posix spawn attributes and actions.
pid_t pid = 0;
posix_spawnattr_t attr = posix_spawnattr_t();
posix_spawn_file_actions_t actions = posix_spawn_file_actions_t();
bool made_it = fork_actions_make_spawn_properties(&attr, &actions, j.get(), dup2s);
if (made_it) {
// We successfully made the attributes and actions; actually call
// posix_spawn.
int spawn_ret =
posix_spawn(&pid, actual_cmd, &actions, &attr, const_cast<char *const *>(argv),
const_cast<char *const *>(envv));
// This usleep can be used to test for various race conditions
// (https://github.com/fish-shell/fish-shell/issues/360).
// usleep(10000);
if (spawn_ret != 0) {
safe_report_exec_error(spawn_ret, actual_cmd, argv, envv);
// Make sure our pid isn't set.
pid = 0;
}
// Clean up our actions.
posix_spawn_file_actions_destroy(&actions);
posix_spawnattr_destroy(&attr);
}
// A 0 pid means we failed to posix_spawn. Since we have no pid, we'll never get
// told when it's exited, so we have to mark the process as failed.
FLOGF(exec_fork, L"Fork #%d, pid %d: spawn external command '%s' from '%ls'",
int(s_fork_count), pid, actual_cmd, file ? file : L"<no file>");
if (pid == 0) {
posix_spawner_t spawner(j.get(), dup2s);
maybe_t<pid_t> pid = spawner.spawn(actual_cmd, const_cast<char *const *>(argv),
const_cast<char *const *>(envv));
if (int err = spawner.get_error()) {
safe_report_exec_error(err, actual_cmd, argv, envv);
job_mark_process_as_failed(j, p);
return false;
}
assert(pid.has_value() && *pid > 0 && "Should have either a valid pid, or an error");
// This usleep can be used to test for various race conditions
// (https://github.com/fish-shell/fish-shell/issues/360).
// usleep(10000);
FLOGF(exec_fork, L"Fork #%d, pid %d: spawn external command '%s' from '%ls'",
int(s_fork_count), *pid, actual_cmd, file ? file : L"<no file>");
// these are all things do_fork() takes care of normally (for forked processes):
p->pid = pid;
p->pid = *pid;
pid_t pgid = maybe_assign_pgid_from_child(j, p->pid);
// posix_spawn should in principle set the pgid before returning.

79
src/postfork.cpp
View File

@ -189,17 +189,35 @@ pid_t execute_fork() {
}
#if FISH_USE_POSIX_SPAWN
bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr,
posix_spawn_file_actions_t *actions, const job_t *j,
const dup2_list_t &dup2s) {
// Initialize the output.
if (posix_spawnattr_init(attr) != 0) {
return false;
// 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_) {
posix_spawnattr_destroy(this->attr());
}
if (actions_) {
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;
}
if (posix_spawn_file_actions_init(actions) != 0) {
posix_spawnattr_destroy(attr);
return false;
{
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.
@ -226,46 +244,45 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr,
if (reset_sigmask) flags |= POSIX_SPAWN_SETSIGMASK;
if (desired_pgid.has_value()) flags |= POSIX_SPAWN_SETPGROUP;
int err = 0;
if (!err) err = posix_spawnattr_setflags(attr, flags);
if (check_fail(posix_spawnattr_setflags(attr(), flags))) return;
if (!err && desired_pgid.has_value()) {
err = posix_spawnattr_setpgroup(attr, *desired_pgid);
if (desired_pgid.has_value()) {
if (check_fail(posix_spawnattr_setpgroup(attr(), *desired_pgid))) return;
}
// Everybody gets default handlers.
if (!err && reset_signal_handlers) {
if (reset_signal_handlers) {
sigset_t sigdefault;
get_signals_with_handlers(&sigdefault);
err = posix_spawnattr_setsigdefault(attr, &sigdefault);
if (check_fail(posix_spawnattr_setsigdefault(attr(), &sigdefault))) return;
}
// No signals blocked.
sigset_t sigmask;
sigemptyset(&sigmask);
if (!err && reset_sigmask) {
if (reset_sigmask) {
sigset_t sigmask;
sigemptyset(&sigmask);
blocked_signals_for_job(*j, &sigmask);
err = posix_spawnattr_setsigmask(attr, &sigmask);
if (check_fail(posix_spawnattr_setsigmask(attr(), &sigmask))) return;
}
// Apply our dup2s.
for (const auto &act : dup2s.get_actions()) {
if (err) break;
if (act.target < 0) {
err = posix_spawn_file_actions_addclose(actions, act.src);
if (check_fail(posix_spawn_file_actions_addclose(actions(), act.src))) return;
} else {
err = posix_spawn_file_actions_adddup2(actions, act.src, act.target);
if (check_fail(posix_spawn_file_actions_adddup2(actions(), act.src, act.target)))
return;
}
}
// Clean up on error.
if (err) {
posix_spawnattr_destroy(attr);
posix_spawn_file_actions_destroy(actions);
}
return !err;
}
maybe_t<pid_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))) return none();
return pid;
}
#endif // FISH_USE_POSIX_SPAWN
void safe_report_exec_error(int err, const char *actual_cmd, const char *const *argv,
@ -385,4 +402,4 @@ static char *get_interpreter(const char *command, char *buffer, size_t buff_size
return buffer + 2;
}
return nullptr;
}
};

40
src/postfork.h
View File

@ -7,6 +7,8 @@
#include <stddef.h>
#include <unistd.h>
#include "maybe.h"
#if HAVE_SPAWN_H
#include <spawn.h>
#endif
@ -51,11 +53,39 @@ void safe_report_exec_error(int err, const char *actual_cmd, const char *const *
const char *const *envv);
#ifdef FISH_USE_POSIX_SPAWN
/// Initializes and fills in a posix_spawnattr_t; on success, the caller should destroy it via
/// posix_spawnattr_destroy.
bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr,
posix_spawn_file_actions_t *actions, const job_t *j,
const dup2_list_t &dup2s);
/// A RAII type which wraps up posix_spawn's data structures.
class posix_spawner_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<pid_t> spawn(const char *cmd, char *const argv[], char *const envp[]);
~posix_spawner_t();
posix_spawner_t(const posix_spawner_t &) = delete;
void operator=(const posix_spawner_t &) = delete;
void operator=(posix_spawner_t &&) = delete;
posix_spawner_t(posix_spawner_t &&) = delete;
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<posix_spawnattr_t> attr_{};
maybe_t<posix_spawn_file_actions_t> actions_{};
};
#endif
#endif