Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-25 12:23:09 +00:00
Code Issues Projects Releases Packages Wiki Activity

Rework file descriptor handling

Browse source 
Remove global array of file descriptors, in
favor of relying on CLO_EXEC exclusively.
Also correctly implement "pipe avoidance" so
that fd redirections do not conflict
with pipes.
This commit is contained in:
ridiculousfish 2015-01-07 18:07:06 -08:00
parent d1feb9bcbf
commit 7864d0d416
6 changed files with 149 additions and 227 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

184
exec.cpp
View file

@ -71,15 +71,6 @@
*/
#define OPEN_MASK 0666
/**
List of all pipes used by internal pipes. These must be closed in
many situations in order to make sure that stray fds aren't lying
around.
Note this is used after fork, so we must not do anything that may allocate memory. Hopefully methods like open_fds.at() don't.
*/
static std::vector<bool> open_fds;
// Called in a forked child
static void exec_write_and_exit(int fd, const char *buff, size_t count, int status)
{
@ -113,12 +104,6 @@ void exec_close(int fd)
break;
}
}
/* Maybe remove this from our set of open fds */
if ((size_t)fd < open_fds.size())
{
open_fds[fd] = false;
}
}
int exec_pipe(int fd[2])
@ -136,15 +121,11 @@ int exec_pipe(int fd[2])
}
debug(4, L"Created pipe using fds %d and %d", fd[0], fd[1]);
int max_fd = std::max(fd[0], fd[1]);
if (max_fd >= 0 && open_fds.size() <= (size_t)max_fd)
{
open_fds.resize(max_fd + 1, false);
}
open_fds.at(fd[0]) = true;
open_fds.at(fd[1]) = true;
// Pipes ought to be cloexec
// Pipes are dup2'd the corresponding fds; the resulting fds are not cloexec
set_cloexec(fd[0]);
set_cloexec(fd[1]);
return res;
}
@ -182,83 +163,6 @@ static bool chain_contains_redirection_to_real_file(const io_chain_t &io_chain)
return result;
}
void print_open_fds(void)
{
for (size_t i=0; i < open_fds.size(); ++i)
{
if (open_fds.at(i))
{
fprintf(stderr, "fd %lu\n", (unsigned long) i);
}
}
}
/**
Check if the specified fd is used as a part of a pipeline in the
specidied set of IO redirections.
This is called after fork().
\param fd the fd to search for
\param io_chain the set of io redirections to search in
*/
static bool use_fd_in_pipe(int fd, const io_chain_t &io_chain)
{
for (size_t idx = 0; idx < io_chain.size(); idx++)
{
const shared_ptr<const io_data_t> &io = io_chain.at(idx);
if ((io->io_mode == IO_BUFFER) ||
(io->io_mode == IO_PIPE))
{
CAST_INIT(const io_pipe_t *, io_pipe, io.get());
if (io_pipe->pipe_fd[0] == fd || io_pipe->pipe_fd[1] == fd)
return true;
}
}
return false;
}
/**
Close all fds in open_fds, except for those that are mentioned in
the redirection list io. This should make sure that there are no
stray opened file descriptors in the child.
\param io the list of io redirections for this job. Pipes mentioned
here should not be closed.
*/
void close_unused_internal_pipes(const io_chain_t &io)
{
/* A call to exec_close will modify open_fds, so be careful how we walk */
for (size_t i=0; i < open_fds.size(); i++)
{
if (open_fds[i])
{
int fd = (int)i;
if (!use_fd_in_pipe(fd, io))
{
debug(4, L"Close fd %d, used in other context", fd);
exec_close(fd);
i--;
}
}
}
}
void get_unused_internal_pipes(std::vector<int> &fds, const io_chain_t &io)
{
for (size_t i=0; i < open_fds.size(); i++)
{
if (open_fds[i])
{
int fd = (int)i;
if (!use_fd_in_pipe(fd, io))
{
fds.push_back(fd);
}
}
}
}
/**
Returns the interpreter for the specified script. Returns NULL if file
is not a script with a shebang.
@ -657,6 +561,24 @@ void exec_job(parser_t &parser, job_t *j)
}
assert(0 && "This should be unreachable");
}
/* We may have block IOs that conflict with fd redirections. For example, we may have a command with a redireciton like <&3; we may also have chosen 3 as the fd for our pipe. Ensure we have no conflicts. */
for (size_t i=0; i < all_ios.size(); i++)
{
io_data_t *io = all_ios.at(i).get();
if (io->io_mode == IO_BUFFER)
{
CAST_INIT(io_buffer_t *, io_buffer, io);
if (! io_buffer->avoid_conflicts_with_io_chain(all_ios))
{
/* We could not avoid conflicts, probably due to fd exhaustion. Mark an error. */
exec_error = true;
job_mark_process_as_failed(j, j->first_process);
break;
}
}
}
signal_block();
@ -670,7 +592,7 @@ void exec_job(parser_t &parser, job_t *j)
exit.
*/
if (job_get_flag(j, JOB_CONTROL))
if (job_get_flag(j, JOB_CONTROL) && ! exec_error)
{
for (const process_t *p = j->first_process; p; p = p->next)
{
@ -734,7 +656,7 @@ void exec_job(parser_t &parser, job_t *j)
*/
int pipe_current_read = -1, pipe_current_write = -1, pipe_next_read = -1;
for (process_t *p=j->first_process; p; p = p->next)
for (process_t *p=j->first_process; p != NULL && ! exec_error; p = p->next)
{
/* The IO chain for this process. It starts with the block IO, then pipes, and then gets any from the process */
io_chain_t process_net_io_chain = j->block_io_chain();
@ -808,10 +730,7 @@ void exec_job(parser_t &parser, job_t *j)
env_export_arr(true);
/*
Set up fd:s that will be used in the pipe
*/
/* Set up fds that will be used in the pipe. */
if (pipes_to_next_command)
{
// debug( 1, L"%ls|%ls" , p->argv[0], p->next->argv[0]);
@ -825,7 +744,19 @@ void exec_job(parser_t &parser, job_t *j)
break;
}
/* Ensure our pipe fds not conflict with any fd redirections. E.g. if the process is like 'cat <&5' then fd 5 must not be used by the pipe. */
if (! pipe_avoid_conflicts_with_io_chain(local_pipe, all_ios))
{
/* We failed. The pipes were closed for us. */
wperror(L"dup");
exec_error = true;
job_mark_process_as_failed(j, p);
break;
}
// This tells the redirection about the fds, but the redirection does not close them
assert(local_pipe[0] >= 0);
assert(local_pipe[1] >= 0);
memcpy(pipe_write->pipe_fd, local_pipe, sizeof(int)*2);
// Record our pipes
@ -837,9 +768,6 @@ void exec_job(parser_t &parser, job_t *j)
pipe_next_read = local_pipe[0];
}
//fprintf(stderr, "before IO: ");
//io_print(j->io);
// This is the IO buffer we use for storing the output of a block or function when it is in a pipeline
shared_ptr<io_buffer_t> block_output_io_buffer;
switch (p->type)
@ -888,7 +816,7 @@ void exec_job(parser_t &parser, job_t *j)
if (p->next)
{
// Be careful to handle failure, e.g. too many open fds
block_output_io_buffer.reset(io_buffer_t::create(STDOUT_FILENO));
block_output_io_buffer.reset(io_buffer_t::create(STDOUT_FILENO, all_ios));
if (block_output_io_buffer.get() == NULL)
{
exec_error = true;
@ -916,7 +844,7 @@ void exec_job(parser_t &parser, job_t *j)
{
if (p->next)
{
block_output_io_buffer.reset(io_buffer_t::create(STDOUT_FILENO));
block_output_io_buffer.reset(io_buffer_t::create(STDOUT_FILENO, all_ios));
if (block_output_io_buffer.get() == NULL)
{
/* We failed (e.g. no more fds could be created). */
@ -939,7 +867,7 @@ void exec_job(parser_t &parser, job_t *j)
case INTERNAL_BUILTIN:
{
int builtin_stdin=0;
int local_builtin_stdin = 0;
bool close_stdin = false;
/*
@ -959,13 +887,13 @@ void exec_job(parser_t &parser, job_t *j)
case IO_FD:
{
CAST_INIT(const io_fd_t *, in_fd, in.get());
builtin_stdin = in_fd->old_fd;
local_builtin_stdin = in_fd->old_fd;
break;
}
case IO_PIPE:
{
CAST_INIT(const io_pipe_t *, in_pipe, in.get());
builtin_stdin = in_pipe->pipe_fd[0];
local_builtin_stdin = in_pipe->pipe_fd[0];
break;
}
@ -973,9 +901,9 @@ void exec_job(parser_t &parser, job_t *j)
{
/* Do not set CLO_EXEC because child needs access */
CAST_INIT(const io_file_t *, in_file, in.get());
builtin_stdin=open(in_file->filename_cstr,
local_builtin_stdin=open(in_file->filename_cstr,
in_file->flags, OPEN_MASK);
if (builtin_stdin == -1)
if (local_builtin_stdin == -1)
{
debug(1,
FILE_ERROR,
@ -999,14 +927,14 @@ void exec_job(parser_t &parser, job_t *j)
really don't do anything. How should this be
handled?
*/
builtin_stdin = -1;
local_builtin_stdin = -1;
break;
}
default:
{
builtin_stdin=-1;
local_builtin_stdin=-1;
debug(1,
_(L"Unknown input redirection type %d"),
in->io_mode);
@ -1018,10 +946,10 @@ void exec_job(parser_t &parser, job_t *j)
}
else
{
builtin_stdin = pipe_read->pipe_fd[0];
local_builtin_stdin = pipe_read->pipe_fd[0];
}
if (builtin_stdin == -1)
if (local_builtin_stdin == -1)
{
exec_error = true;
break;
@ -1046,7 +974,7 @@ void exec_job(parser_t &parser, job_t *j)
to make exec handle things.
*/
builtin_push_io(parser, builtin_stdin);
builtin_push_io(parser, local_builtin_stdin);
builtin_out_redirect = has_fd(process_net_io_chain, STDOUT_FILENO);
builtin_err_redirect = has_fd(process_net_io_chain, STDERR_FILENO);
@ -1055,7 +983,7 @@ void exec_job(parser_t &parser, job_t *j)
job_set_flag(j, JOB_FOREGROUND, 0);
signal_unblock();
p->status = builtin_run(parser, p->get_argv(), process_net_io_chain);
builtin_out_redirect=old_out;
@ -1077,7 +1005,7 @@ void exec_job(parser_t &parser, job_t *j)
*/
if (close_stdin)
{
exec_close(builtin_stdin);
exec_close(local_builtin_stdin);
}
break;
}
@ -1284,7 +1212,6 @@ void exec_job(parser_t &parser, job_t *j)
if (g_log_forks)
{
printf("fork #%d: Executing fork for internal builtin for '%ls'\n", g_fork_count, p->argv0());
io_print(process_net_io_chain);
}
pid = execute_fork(false);
if (pid == 0)
@ -1329,15 +1256,12 @@ void exec_job(parser_t &parser, job_t *j)
std::string actual_cmd_str = wcs2string(p->actual_cmd);
const char *actual_cmd = actual_cmd_str.c_str();
const wchar_t *reader_current_filename(void);
if (g_log_forks)
{
const wchar_t *file = reader_current_filename();
printf("fork #%d: forking for '%s' in '%ls'\n", g_fork_count, actual_cmd, file ? file : L"");
fprintf(stderr, "IO chain for %s:\n", actual_cmd);
io_print(process_net_io_chain);
}
#if FISH_USE_POSIX_SPAWN
@ -1501,7 +1425,7 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, boo
int subcommand_status = -1; //assume the worst
// IO buffer creation may fail (e.g. if we have too many open files to make a pipe), so this may be null
const shared_ptr<io_buffer_t> io_buffer(io_buffer_t::create(STDOUT_FILENO));
const shared_ptr<io_buffer_t> io_buffer(io_buffer_t::create(STDOUT_FILENO, io_chain_t()));
if (io_buffer.get() != NULL)
{
parser_t &parser = parser_t::principal_parser();

12
exec.h
View file

@ -60,22 +60,16 @@ int exec_subshell(const wcstring &cmd, bool preserve_exit_status);
/**
Loops over close until the syscall was run without being
interrupted. Then removes the fd from the open_fds list.
interrupted.
*/
void exec_close(int fd);
/**
Call pipe(), and add resulting fds to open_fds, the list of opend
file descriptors for pipes.
Call pipe(), and add resulting fds to open_fds, the list of opened
file descriptors for pipes. The pipes are marked CLO_EXEC.
*/
int exec_pipe(int fd[2]);
/* Close all fds in open_fds. This is called from postfork.cpp */
void close_unused_internal_pipes(const io_chain_t &io);
/* Gets all unused internal pipes into fds */
void get_unused_internal_pipes(std::vector<int> &fds, const io_chain_t &io);
/** Gets the interpreter for a given command */
char *get_interpreter(const char *command, char *interpreter, size_t buff_size);

2
fish_tests.cpp
View file

@ -773,7 +773,7 @@ static int signal_main(test_cancellation_info_t *info)
static void test_1_cancellation(const wchar_t *src)
{
shared_ptr<io_buffer_t> out_buff(io_buffer_t::create(STDOUT_FILENO));
shared_ptr<io_buffer_t> out_buff(io_buffer_t::create(STDOUT_FILENO, io_chain_t()));
const io_chain_t io_chain(out_buff);
test_cancellation_info_t ctx = {pthread_self(), 0.25 /* seconds */ };
iothread_perform(signal_main, (void (*)(test_cancellation_info_t *, int))NULL, &ctx);

97
io.cpp
View file

@ -124,8 +124,17 @@ void io_buffer_t::read()
}
}
bool io_buffer_t::avoid_conflicts_with_io_chain(const io_chain_t &ios)
{
bool result = pipe_avoid_conflicts_with_io_chain(this->pipe_fd, ios);
if (! result)
{
wperror(L"dup");
}
return result;
}
io_buffer_t *io_buffer_t::create(int fd)
io_buffer_t *io_buffer_t::create(int fd, const io_chain_t &conflicts)
{
bool success = true;
assert(fd >= 0);
@ -137,6 +146,11 @@ io_buffer_t *io_buffer_t::create(int fd)
wperror(L"pipe");
success = false;
}
else if (! buffer_redirect->avoid_conflicts_with_io_chain(conflicts))
{
// The above call closes the fds on error
success = false;
}
else if (make_fd_nonblocking(buffer_redirect->pipe_fd[0]) != 0)
{
debug(1, PIPE_ERROR);
@ -149,27 +163,11 @@ io_buffer_t *io_buffer_t::create(int fd)
delete buffer_redirect;
buffer_redirect = NULL;
}
else
{
//fprintf(stderr, "Created pipes {%d, %d} for %p\n", buffer_redirect->pipe_fd[0], buffer_redirect->pipe_fd[1], buffer_redirect);
}
return buffer_redirect;
}
io_buffer_t::~io_buffer_t()
{
//fprintf(stderr, "Deallocating pipes {%d, %d} for %p\n", this->pipe_fd[0], this->pipe_fd[1], this);
/**
If this is an input buffer, then io_read_buffer will not have
been called, and we need to close the output fd as well.
*/
if (is_input && pipe_fd[1] >= 0)
{
exec_close(pipe_fd[1]);
}
if (pipe_fd[0] >= 0)
{
exec_close(pipe_fd[0]);
@ -236,6 +234,71 @@ void io_print(const io_chain_t &chain)
}
}
/* If the given fd is used by the io chain, duplicates it repeatedly until an fd not used in the io chain is found, or we run out. If we return a new fd or an error, closes the old one. Any fd created is marked close-on-exec. Returns -1 on failure (in which case the given fd is still closed). */
static int move_fd_to_unused(int fd, const io_chain_t &io_chain)
{
int new_fd = fd;
if (fd >= 0 && io_chain.get_io_for_fd(fd).get() != NULL)
{
/* We have fd >= 0, and it's a conflict. dup it and recurse. Note that we recurse before anything is closed; this forces the kernel to give us a new one (or report fd exhaustion). */
int tmp_fd;
do
{
tmp_fd = dup(fd);
} while (tmp_fd < 0 && errno == EINTR);
assert(tmp_fd != fd);
if (tmp_fd < 0)
{
/* Likely fd exhaustion. */
new_fd = -1;
}
else
{
/* Ok, we have a new candidate fd. Recurse. If we get a valid fd, either it's the same as what we gave it, or it's a new fd and what we gave it has been closed. If we get a negative value, the fd also has been closed. */
set_cloexec(tmp_fd);
new_fd = move_fd_to_unused(tmp_fd, io_chain);
}
/* We're either returning a new fd or an error. In both cases, we promise to close the old one. */
assert(new_fd != fd);
int saved_errno = errno;
exec_close(fd);
errno = saved_errno;
}
return new_fd;
}
bool pipe_avoid_conflicts_with_io_chain(int fds[2], const io_chain_t &ios)
{
bool success = true;
for (int i=0; i < 2; i++)
{
fds[i] = move_fd_to_unused(fds[i], ios);
if (fds[i] < 0)
{
success = false;
break;
}
}
/* If any fd failed, close all valid fds */
if (! success)
{
int saved_errno = errno;
for (int i=0; i < 2; i++)
{
if (fds[i] >= 0)
{
exec_close(fds[i]);
fds[i] = -1;
}
}
errno = saved_errno;
}
return success;
}
/* Return the last IO for the given fd */
shared_ptr<const io_data_t> io_chain_t::get_io_for_fd(int fd) const
{

12
io.h
View file

@ -122,6 +122,7 @@ public:
}
};
class io_chain_t;
class io_buffer_t : public io_pipe_t
{
private:
@ -161,6 +162,9 @@ public:
{
return out_buffer.size();
}
/* Ensures that the pipes do not conflict with any fd redirections in the chain */
bool avoid_conflicts_with_io_chain(const io_chain_t &ios);
/**
Close output pipe, and read from input pipe until eof.
@ -170,11 +174,13 @@ public:
/**
Create a IO_BUFFER type io redirection, complete with a pipe and a
vector<char> for output. The default file descriptor used is STDOUT_FILENO
for buffering
for buffering.
\param fd the fd that will be mapped in the child process, typically STDOUT_FILENO
\param conflicts A set of IO redirections. The function ensures that any pipe it makes
does not conflict with an fd redirection in this list.
*/
static io_buffer_t *create(int fd);
static io_buffer_t *create(int fd, const io_chain_t &conflicts);
};
class io_chain_t : public std::vector<shared_ptr<io_data_t> >
@ -199,6 +205,8 @@ public:
shared_ptr<const io_data_t> io_chain_get(const io_chain_t &src, int fd);
shared_ptr<io_data_t> io_chain_get(io_chain_t &src, int fd);
/* Given a pair of fds, if an fd is used by the given io chain, duplicate that fd repeatedly until we find one that does not conflict, or we run out of fds. Returns the new fds by reference, closing the old ones. If we get an error, returns false (in which case both fds are closed and set to -1). */
bool pipe_avoid_conflicts_with_io_chain(int fds[2], const io_chain_t &ios);
/** Print debug information about the specified IO redirection chain to stderr. */
void io_print(const io_chain_t &chain);

69
postfork.cpp
View file

@ -107,53 +107,6 @@ int set_child_group(job_t *j, process_t *p, int print_errors)
return res;
}
/** Make sure the fd used by each redirection is not used by a pipe. Note that while this does not modify the vector, it does modify the IO redirections within (gulp) */
static void free_redirected_fds_from_pipes(const io_chain_t &io_chain)
{
size_t max = io_chain.size();
for (size_t i = 0; i < max; i++)
{
int fd_to_free = io_chain.at(i)->fd;
/* We only have to worry about fds beyond the three standard ones */
if (fd_to_free <= 2)
continue;
/* Make sure the fd is not used by a pipe */
for (size_t j = 0; j < max; j++)
{
/* We're only interested in pipes */
io_data_t *io = io_chain.at(j).get();
if (io->io_mode != IO_PIPE && io->io_mode != IO_BUFFER)
continue;
CAST_INIT(io_pipe_t *, possible_conflict, io);
/* If the pipe is a conflict, dup it to some other value */
for (int k=0; k<2; k++)
{
/* If it's not a conflict, we don't care */
if (possible_conflict->pipe_fd[k] != fd_to_free)
continue;
/* Repeat until we have a replacement fd */
int replacement_fd = -1;
while (replacement_fd < 0)
{
replacement_fd = dup(fd_to_free);
if (replacement_fd == -1 && errno != EINTR)
{
debug_safe_int(1, FD_ERROR, fd_to_free);
safe_perror("dup");
FATAL_EXIT();
}
}
possible_conflict->pipe_fd[k] = replacement_fd;
}
}
}
}
/**
Set up a childs io redirections. Should only be called by
setup_child_process(). Does the following: First it closes any open
@ -168,9 +121,6 @@ static void free_redirected_fds_from_pipes(const io_chain_t &io_chain)
*/
static int handle_child_io(const io_chain_t &io_chain)
{
//fprintf(stderr, "child IO for %d\n", getpid());
close_unused_internal_pipes(io_chain);
free_redirected_fds_from_pipes(io_chain);
for (size_t idx = 0; idx < io_chain.size(); idx++)
{
const io_data_t *io = io_chain.at(idx).get();
@ -442,18 +392,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, posix_spawn_fil
sigemptyset(&sigmask);
if (! err && reset_sigmask)
err = posix_spawnattr_setsigmask(attr, &sigmask);
/* Make sure that our pipes don't use an fd that the redirection itself wants to use */
free_redirected_fds_from_pipes(io_chain);
/* Close unused internal pipes */
std::vector<int> files_to_close;
get_unused_internal_pipes(files_to_close, io_chain);
for (size_t i = 0; ! err && i < files_to_close.size(); i++)
{
err = posix_spawn_file_actions_addclose(actions, files_to_close.at(i));
}
for (size_t idx = 0; idx < io_chain.size(); idx++)
{
const shared_ptr<const io_data_t> io = io_chain.at(idx);
@ -465,12 +404,6 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, posix_spawn_fil
continue;
}
if (io->fd > 2)
{
/* Make sure the fd used by this redirection is not used by e.g. a pipe. */
// free_fd(io_chain, io->fd );
// PCA I don't think we need to worry about this. fd redirection is pretty uncommon anyways.
}
switch (io->io_mode)
{
case IO_CLOSE: