Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-11-15 09:27:38 +00:00
Code Issues Projects Releases Packages Wiki Activity

Adopt dup2_list_t in fork execution path

Browse source 
This switches IO redirections after fork() to use the dup2_list_t,
instead of io_chain_t. This results in simpler code with much simpler
error handling.
This commit is contained in:
ridiculousfish 2019-01-28 14:35:56 -08:00
parent 6c22c8893b
commit d62576ce22
4 changed files with 42 additions and 143 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
src/exec.cpp
View file

@ -39,6 +39,7 @@
#include "postfork.h"
#include "proc.h"
#include "reader.h"
#include "redirection.h"
#include "signal.h"
#include "wutil.h" // IWYU pragma: keep
@ -367,7 +368,8 @@ void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &all_ios) {
// It's known to be wrong - for example, it means that redirections bound for subsequent
// commands in the pipeline will apply to exec. However, using exec in a pipeline doesn't
// really make sense, so I'm not trying to fix it here.
if (!setup_child_process(0, all_ios)) {
auto redirs = dup2_list_t::resolve_chain(all_ios);
if (redirs && !setup_child_process(0, *redirs)) {
// Decrement SHLVL as we're removing ourselves from the shell "stack".
auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT);
wcstring shlvl_str = L"0";
@ -404,7 +406,7 @@ static void on_process_created(const std::shared_ptr<job_t> &j, pid_t child_pid)
/// Call fork() as part of executing a process \p p in a job \j. Execute \p child_action in the
/// context of the child. Returns true if fork succeeded, false if fork failed.
static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t *p,
const io_chain_t &io_chain, bool drain_threads,
const dup2_list_t &dup2s, bool drain_threads,
const char *fork_type,
const std::function<void()> &child_action) {
pid_t pid = execute_fork(drain_threads);
@ -413,7 +415,7 @@ static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t
// stdout and stderr, and then exit.
p->pid = getpid();
child_set_group(job.get(), p);
setup_child_process(p, io_chain);
setup_child_process(p, dup2s);
child_action();
DIE("Child process returned control to fork_child lambda!");
}
@ -634,9 +636,15 @@ static bool handle_builtin_output(const std::shared_ptr<job_t> &j, process_t *p,
const char *errbuff = errbuff_str.data();
size_t errbuff_len = errbuff_str.size();
// Resolve our IO chain to a sequence of dup2s.
auto dup2s = dup2_list_t::resolve_chain(*io_chain);
if (!dup2s) {
return false;
}
fflush(stdout);
fflush(stderr);
if (!fork_child_for_process(j, p, *io_chain, false, "internal builtin", [&] {
if (!fork_child_for_process(j, p, *dup2s, false, "internal builtin", [&] {
do_builtin_io(outbuff, outbuff_len, errbuff, errbuff_len);
exit_without_destructors(p->status);
})) {
@ -655,6 +663,11 @@ static bool exec_external_command(env_stack_t &vars, const std::shared_ptr<job_t
null_terminated_array_t<char> argv_array;
convert_wide_array_to_narrow(p->get_argv_array(), &argv_array);
// Convert our IO chain to a dup2 sequence.
auto dup2s = dup2_list_t::resolve_chain(proc_io_chain);
if (! dup2s)
return false;
// Ensure that stdin is blocking before we hand it off (see issue #176). It's a
// little strange that we only do this with stdin and not with stdout or stderr.
// However in practice, setting or clearing O_NONBLOCK on stdin also sets it for the
@ -741,7 +754,7 @@ static bool exec_external_command(env_stack_t &vars, const std::shared_ptr<job_t
} else
#endif
{
if (!fork_child_for_process(j, p, proc_io_chain, false, "external command",
if (!fork_child_for_process(j, p, *dup2s, false, "external command",
[&] { safe_launch_process(p, actual_cmd, argv, envv); })) {
return false;
}
@ -820,6 +833,12 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
io_chain.remove(block_output_io_buffer);
block_output_io_buffer->read();
// Resolve our IO chain to a sequence of dup2s.
auto dup2s = dup2_list_t::resolve_chain(io_chain);
if (!dup2s) {
return false;
}
const std::string buffer_contents = block_output_io_buffer->buffer().newline_serialized();
const char *buffer = buffer_contents.data();
size_t count = buffer_contents.size();
@ -827,7 +846,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
// We don't have to drain threads here because our child process is simple.
const char *fork_reason =
p->type == INTERNAL_BLOCK_NODE ? "internal block io" : "internal function io";
if (!fork_child_for_process(j, p, io_chain, false, fork_reason, [&] {
if (!fork_child_for_process(j, p, *dup2s, false, fork_reason, [&] {
exec_write_and_exit(block_output_io_buffer->fd, buffer, count, status);
})) {
return false;

147
src/postfork.cpp
View file

@ -19,6 +19,7 @@
#include "iothread.h"
#include "postfork.h"
#include "proc.h"
#include "redirection.h"
#include "signal.h"
#include "wutil.h" // IWYU pragma: keep
@ -38,27 +39,6 @@
/// Fork error message.
#define FORK_ERROR "Could not create child process - exiting"
/// File redirection clobbering error message.
#define NOCLOB_ERROR "The file '%s' already exists"
/// File redirection error message.
#define FILE_ERROR "An error occurred while redirecting file '%s'"
/// File descriptor redirection error message.
#define FD_ERROR "An error occurred while redirecting file descriptor %s"
/// Pipe error message.
#define LOCAL_PIPE_ERROR "An error occurred while setting up pipe"
static bool log_redirections = false;
/// Cover for debug_safe that can take an int. The format string should expect a %s.
static void debug_safe_int(int level, const char *format, int val) {
char buff[128];
format_long_safe(buff, val);
debug_safe(level, format, buff);
}
/// Called only by the child to set its own process group (possibly creating a new group in the
/// process if it is the first in a JOB_CONTROL job.
/// Returns true on sucess, false on failiure.
@ -175,126 +155,23 @@ bool maybe_assign_terminal(const job_t *j) {
return true;
}
/// Set up a childs io redirections. Should only be called by setup_child_process(). Does the
/// following: First it closes any open file descriptors not related to the child by calling
/// close_unused_internal_pipes() and closing the universal variable server file descriptor. It then
/// goes on to perform all the redirections described by \c io.
///
/// \param io_chain the list of IO redirections for the child
///
/// \return 0 on sucess, -1 on failure
static int handle_child_io(const io_chain_t &io_chain) {
for (size_t idx = 0; idx < io_chain.size(); idx++) {
const io_data_t *io = io_chain.at(idx).get();
if (io->io_mode == io_mode_t::fd && io->fd == static_cast<const io_fd_t *>(io)->old_fd) {
continue;
}
switch (io->io_mode) {
case io_mode_t::close: {
if (log_redirections) fwprintf(stderr, L"%d: close %d\n", getpid(), io->fd);
if (close(io->fd)) {
debug_safe_int(0, "Failed to close file descriptor %s", io->fd);
safe_perror("close");
}
break;
}
case io_mode_t::file: {
// Here we definitely do not want to set CLO_EXEC because our child needs access.
const io_file_t *io_file = static_cast<const io_file_t *>(io);
int tmp = open(io_file->filename_cstr, io_file->flags, OPEN_MASK);
if (tmp < 0) {
if ((io_file->flags & O_EXCL) && (errno == EEXIST)) {
debug_safe(1, NOCLOB_ERROR, io_file->filename_cstr);
} else {
debug_safe(1, FILE_ERROR, io_file->filename_cstr);
safe_perror("open");
}
return -1;
} else if (tmp != io->fd) {
// This call will sometimes fail, but that is ok, this is just a precausion.
close(io->fd);
if (dup2(tmp, io->fd) == -1) {
debug_safe_int(1, FD_ERROR, io->fd);
safe_perror("dup2");
exec_close(tmp);
return -1;
}
exec_close(tmp);
}
break;
}
case io_mode_t::fd: {
int old_fd = static_cast<const io_fd_t *>(io)->old_fd;
if (log_redirections)
fwprintf(stderr, L"%d: fd dup %d to %d\n", getpid(), old_fd, io->fd);
// This call will sometimes fail, but that is ok, this is just a precausion.
close(io->fd);
if (dup2(old_fd, io->fd) == -1) {
debug_safe_int(1, FD_ERROR, io->fd);
safe_perror("dup2");
return -1;
}
break;
}
case io_mode_t::buffer:
case io_mode_t::pipe: {
const io_pipe_t *io_pipe = static_cast<const io_pipe_t *>(io);
// If write_pipe_idx is 0, it means we're connecting to the read end (first pipe
// fd). If it's 1, we're connecting to the write end (second pipe fd).
unsigned int write_pipe_idx = (io_pipe->is_input ? 0 : 1);
#if 0
debug(0, L"%ls %ls on fd %d (%d %d)", write_pipe?L"write":L"read",
(io->io_mode == io_mode_t::buffer)?L"buffer":L"pipe", io->fd, io->pipe_fd[0],
io->pipe_fd[1]);
#endif
if (log_redirections)
fwprintf(stderr, L"%d: %s dup %d to %d\n", getpid(),
io->io_mode == io_mode_t::buffer ? "buffer" : "pipe",
io_pipe->pipe_fd[write_pipe_idx], io->fd);
if (dup2(io_pipe->pipe_fd[write_pipe_idx], io->fd) != io->fd) {
debug_safe(1, LOCAL_PIPE_ERROR);
safe_perror("dup2");
return -1;
}
if (io_pipe->pipe_fd[0] >= 0) exec_close(io_pipe->pipe_fd[0]);
if (io_pipe->pipe_fd[1] >= 0) exec_close(io_pipe->pipe_fd[1]);
break;
int setup_child_process(process_t *p, const dup2_list_t &dup2s) {
for (const auto &act : dup2s.get_actions()) {
int err = act.target < 0 ? close(act.src) : dup2(act.src, act.target);
if (err < 0) {
// We have a null p if this is for the exec (non-fork) path.
if (p != nullptr) {
debug_safe(4, "redirect_in_child_after_fork failed in setup_child_process");
exit_without_destructors(1);
}
return err;
}
}
// Set the handling for job control signals back to the default.
signal_reset_handlers();
return 0;
}
int setup_child_process(process_t *p, const io_chain_t &io_chain) {
bool ok = true;
if (ok) {
// In the case of io_mode_t::file, this can hang until data is available to read/write!
ok = (0 == handle_child_io(io_chain));
if (p != 0 && !ok) {
debug_safe(4, "handle_child_io failed in setup_child_process");
exit_without_destructors(1);
}
}
if (ok) {
// Set the handling for job control signals back to the default.
signal_reset_handlers();
}
return ok ? 0 : -1;
}
int g_fork_count = 0;

5
src/postfork.h
View file

@ -14,6 +14,7 @@
#define FISH_USE_POSIX_SPAWN HAVE_SPAWN_H
#endif
class dup2_list_t;
class io_chain_t;
class job_t;
class process_t;
@ -29,11 +30,11 @@ bool maybe_assign_terminal(const job_t *j);
/// descriptor actions are performed.
///
/// \param p the child process to set up
/// \param io_chain the IO chain to use
/// \param dup2 the dup2 list to apply
///
/// \return 0 on sucess, -1 on failiure. When this function returns, signals are always unblocked.
/// On failiure, signal handlers, io redirections and process group of the process is undefined.
int setup_child_process(process_t *p, const io_chain_t &io_chain);
int setup_child_process(process_t *p, const dup2_list_t &dup2s);
/// Call fork(), optionally waiting until we are no longer multithreaded. If the forked child
/// doesn't do anything that could allocate memory, take a lock, etc. (like call exec), then it's

2
src/redirection.h
View file

@ -11,6 +11,7 @@
/// A class representing a sequence of basic redirections.
class dup2_list_t {
public:
/// A type that represents the action dup2(src, target).
/// If target is negative, this represents close(src).
/// Note none of the fds here are considered 'owned'.
@ -19,6 +20,7 @@ class dup2_list_t {
int target;
};
private:
/// The list of actions.
std::vector<action_t> actions_;