Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2025-01-13 21:44:16 +00:00
Code Issues Projects Releases Packages Wiki Activity

Switch a job's process list from a linked list to a vector of pointers

Browse source 
Clarifies and simplifies the memory management around process handling.
This commit is contained in:
ridiculousfish 2017-01-23 09:28:34 -08:00
parent f4476100f2
commit ab189a75ab
7 changed files with 79 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/builtin_jobs.cpp
View file

@ -28,13 +28,12 @@ enum {
/// Calculates the cpu usage (in percent) of the specified job.
static int cpu_use(const job_t *j) {
double u = 0;
process_t *p;
for (p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
struct timeval t;
int jiffies;
gettimeofday(&t, 0);
jiffies = proc_get_jiffies(p);
jiffies = proc_get_jiffies(p.get());
double t1 = 1000000.0 * p->last_time.tv_sec + p->last_time.tv_usec;
double t2 = 1000000.0 * t.tv_sec + t.tv_usec;
@ -48,7 +47,6 @@ static int cpu_use(const job_t *j) {
/// Print information about the specified job.
static void builtin_jobs_print(const job_t *j, int mode, int header, io_streams_t &streams) {
process_t *p;
switch (mode) {
case JOBS_DEFAULT: {
if (header) {
@ -85,7 +83,7 @@ static void builtin_jobs_print(const job_t *j, int mode, int header, io_streams_
streams.out.append(_(L"Process\n"));
}
for (p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
streams.out.append_format(L"%d\n", p->pid);
}
break;
@ -96,7 +94,7 @@ static void builtin_jobs_print(const job_t *j, int mode, int header, io_streams_
streams.out.append(_(L"Command\n"));
}
for (p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
streams.out.append_format(L"%ls\n", p->argv0());
}
break;

44
src/exec.cpp
View file

@ -397,7 +397,7 @@ void exec_job(parser_t &parser, job_t *j) {
}
}
if (j->first_process->type == INTERNAL_EXEC) {
if (j->processes.front()->type == INTERNAL_EXEC) {
// Do a regular launch - but without forking first...
signal_block();
@ -421,10 +421,10 @@ void exec_job(parser_t &parser, job_t *j) {
env_set(L"SHLVL", nshlvl_str.c_str(), ENV_GLOBAL | ENV_EXPORT);
// launch_process _never_ returns.
launch_process_nofork(j->first_process);
launch_process_nofork(j->processes.front().get());
} else {
job_set_flag(j, JOB_CONSTRUCTED, 1);
j->first_process->completed = 1;
j->processes.front()->completed = 1;
return;
}
DIE("this should be unreachable");
@ -440,7 +440,7 @@ void exec_job(parser_t &parser, job_t *j) {
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);
job_mark_process_as_failed(j, j->processes.front().get());
break;
}
}
@ -453,13 +453,9 @@ void exec_job(parser_t &parser, job_t *j) {
// builtin/block/function is inside a pipeline, since that usually means we have to wait for one
// program to exit before continuing in the pipeline, causing the group leader to exit.
if (job_get_flag(j, JOB_CONTROL) && !exec_error) {
for (const process_t *p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
if (p->type != EXTERNAL) {
if (p->next) {
needs_keepalive = true;
break;
}
if (p != j->first_process) {
if (! p->is_last_in_job || ! p->is_first_in_job) {
needs_keepalive = true;
break;
}
@ -498,7 +494,11 @@ void exec_job(parser_t &parser, job_t *j) {
// We are careful to set these to -1 when closed, so if we exit the loop abruptly, we can still
// close them.
int pipe_current_read = -1, pipe_current_write = -1, pipe_next_read = -1;
for (process_t *p = j->first_process; p != NULL && !exec_error; p = p->next) {
for (std::unique_ptr<process_t> &unique_p : j->processes) {
if (exec_error) {
break;
}
process_t * const p = unique_p.get();
// 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();
@ -509,7 +509,7 @@ void exec_job(parser_t &parser, job_t *j) {
pipe_next_read = -1;
// See if we need a pipe.
const bool pipes_to_next_command = (p->next != NULL);
const bool pipes_to_next_command = ! p->is_last_in_job;
// The pipes the current process write to and read from. Unfortunately these can't be just
// allocated on the stack, since j->io wants shared_ptr.
@ -545,7 +545,7 @@ void exec_job(parser_t &parser, job_t *j) {
shared_ptr<io_pipe_t> pipe_read;
// Write pipe goes first.
if (p->next) {
if (pipes_to_next_command) {
pipe_write.reset(new io_pipe_t(p->pipe_write_fd, false));
process_net_io_chain.push_back(pipe_write);
}
@ -554,7 +554,7 @@ void exec_job(parser_t &parser, job_t *j) {
process_net_io_chain.append(p->io_chain());
// Read pipe goes last.
if (p != j->first_process) {
if (! p->is_first_in_job) {
pipe_read.reset(new io_pipe_t(p->pipe_read_fd, true));
// Record the current read in pipe_read.
pipe_read->pipe_fd[0] = pipe_current_read;
@ -640,7 +640,7 @@ void exec_job(parser_t &parser, job_t *j) {
parser.forbid_function(func_name);
if (p->next) {
if (! p->is_last_in_job) {
// Be careful to handle failure, e.g. too many open fds.
block_output_io_buffer = io_buffer_t::create(STDOUT_FILENO, all_ios);
if (block_output_io_buffer.get() == NULL) {
@ -667,7 +667,7 @@ void exec_job(parser_t &parser, job_t *j) {
}
case INTERNAL_BLOCK_NODE: {
if (p->next) {
if (! p->is_last_in_job) {
block_output_io_buffer = 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).
@ -693,7 +693,7 @@ void exec_job(parser_t &parser, job_t *j) {
// If this is the first process, check the io redirections and see where we should
// be reading from.
if (p == j->first_process) {
if (p->is_first_in_job) {
const shared_ptr<const io_data_t> in =
process_net_io_chain.get_io_for_fd(STDIN_FILENO);
@ -757,7 +757,7 @@ void exec_job(parser_t &parser, job_t *j) {
} else {
// Determine if we have a "direct" redirection for stdin.
bool stdin_is_directly_redirected;
if (p != j->first_process) {
if (! p->is_first_in_job) {
// We must have a pipe
stdin_is_directly_redirected = true;
} else {
@ -831,7 +831,7 @@ void exec_job(parser_t &parser, job_t *j) {
if (!block_output_io_buffer.get()) {
// No buffer, so we exit directly. This means we have to manually set the exit
// status.
if (p->next == NULL) {
if (p->is_last_in_job) {
proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? (!status) : status);
}
p->completed = 1;
@ -866,7 +866,7 @@ void exec_job(parser_t &parser, job_t *j) {
}
} else {
if (p->next == 0) {
if (p->is_last_in_job) {
proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? (!status) : status);
}
p->completed = 1;
@ -896,7 +896,7 @@ void exec_job(parser_t &parser, job_t *j) {
// output, so that we can truncate the file. Does not apply to /dev/null.
bool must_fork = redirection_is_to_real_file(stdout_io.get()) ||
redirection_is_to_real_file(stderr_io.get());
if (!must_fork && p->next == NULL) {
if (!must_fork && p->is_last_in_job) {
const bool stdout_is_to_buffer = stdout_io && stdout_io->io_mode == IO_BUFFER;
const bool no_stdout_output = stdout_buffer.empty();
const bool no_stderr_output = stderr_buffer.empty();
@ -939,7 +939,7 @@ void exec_job(parser_t &parser, job_t *j) {
if (fork_was_skipped) {
p->completed = 1;
if (p->next == 0) {
if (p->is_last_in_job) {
debug(3, L"Set status of %ls to %d using short circuit", j->command_wcstr(),
p->status);

3
src/expand.cpp
View file

@ -519,9 +519,8 @@ static int find_job(const struct find_job_data_t *info) {
jobs.reset();
while ((j = jobs.next())) {
process_t *p;
if (j->command_is_empty()) continue;
for (p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
if (p->actual_cmd.empty()) continue;
size_t offset;

28
src/parse_execution.cpp
View file

@ -1146,9 +1146,9 @@ parse_execution_result_t parse_execution_context_t::populate_job_from_job_node(
parse_execution_result_t result = parse_execution_success;
// Create processes. Each one may fail.
std::vector<process_t *> processes;
processes.push_back(new process_t());
result = this->populate_job_process(j, processes.back(), *statement_node);
process_list_t processes;
processes.emplace_back(new process_t());
result = this->populate_job_process(j, processes.back().get(), *statement_node);
// Construct process_ts for job continuations (pipelines), by walking the list until we hit the
// terminal (empty) job continuation.
@ -1171,29 +1171,23 @@ parse_execution_result_t parse_execution_context_t::populate_job_from_job_node(
assert(statement_node != NULL);
// Store the new process (and maybe with an error).
processes.push_back(new process_t());
result = this->populate_job_process(j, processes.back(), *statement_node);
processes.emplace_back(new process_t());
result = this->populate_job_process(j, processes.back().get(), *statement_node);
// Get the next continuation.
job_cont = get_child(*job_cont, 2, symbol_job_continuation);
assert(job_cont != NULL);
}
// Inform our processes of who is first and last
processes.front()->is_first_in_job = true;
processes.back()->is_last_in_job = true;
// Return what happened.
if (result == parse_execution_success) {
// Link up the processes.
assert(!processes.empty()); //!OCLINT(multiple unary operator)
j->first_process = processes.at(0);
for (size_t i = 1; i < processes.size(); i++) {
processes.at(i - 1)->next = processes.at(i);
}
} else {
// Clean up processes.
for (size_t i = 0; i < processes.size(); i++) {
const process_t *proc = processes.at(i);
processes.at(i) = NULL;
delete proc;
}
j->processes = std::move(processes);
}
return result;
}
@ -1313,7 +1307,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
// Check to see if this contained any external commands.
bool job_contained_external_command = false;
for (const process_t *proc = j->first_process; proc != NULL; proc = proc->next) {
for (const auto &proc : j->processes) {
if (proc->type == EXTERNAL) {
job_contained_external_command = true;
break;

2
src/parser.cpp
View file

@ -525,7 +525,7 @@ wcstring parser_t::current_line() {
void parser_t::job_add(job_t *job) {
assert(job != NULL);
assert(job->first_process != NULL);
assert(! job->processes.empty());
this->my_job_list.push_front(job);
}

67
src/proc.cpp
View file

@ -214,9 +214,7 @@ job_t *job_get_from_pid(int pid) {
///
/// \param j the job to test
int job_is_stopped(const job_t *j) {
process_t *p;
for (p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
if (!p->completed && !p->stopped) {
return 0;
}
@ -228,9 +226,9 @@ int job_is_stopped(const job_t *j) {
///
/// \param j the job to test
bool job_is_completed(const job_t *j) {
assert(j->first_process != NULL);
assert(! j->processes.empty());
bool result = true;
for (process_t *p = j->first_process; p != NULL; p = p->next) {
for (const process_ptr_t &p : j->processes) {
if (!p->completed) {
result = false;
break;
@ -256,7 +254,7 @@ int job_signal(job_t *j, int signal) {
if (j->pgid != my_pid) {
res = killpg(j->pgid, signal);
} else {
for (process_t *p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
if (!p->completed && p->pid && kill(p->pid, signal)) {
res = -1;
break;
@ -284,12 +282,15 @@ static void mark_process_status(process_t *p, int status) {
}
}
void job_mark_process_as_failed(const job_t *job, process_t *p) {
void job_mark_process_as_failed(job_t *job, const process_t *failed_proc) {
// The given process failed to even lift off (e.g. posix_spawn failed) and so doesn't have a
// valid pid. Mark it as dead.
UNUSED(job);
for (process_t *cursor = p; cursor != NULL; cursor = cursor->next) {
cursor->completed = 1;
// valid pid. Mark it and everything after it as dead.
bool found = false;
for (process_ptr_t &p : job->processes) {
found = found || (p.get() == failed_proc);
if (found) {
p->completed = true;
}
}
}
@ -299,22 +300,22 @@ void job_mark_process_as_failed(const job_t *job, process_t *p) {
/// \param status the status as returned by wait
static void handle_child_status(pid_t pid, int status) {
bool found_proc = false;
const job_t *j = NULL;
job_t *j = NULL;
process_t *p = NULL;
job_iterator_t jobs;
while (!found_proc && (j = jobs.next())) {
process_t *prev = 0;
for (p = j->first_process; p; p = p->next) {
process_t *prev = NULL;
for (process_ptr_t &p : j->processes) {
if (pid == p->pid) {
mark_process_status(p, status);
mark_process_status(p.get(), status);
if (p->completed && prev && !prev->completed && prev->pid) {
kill(prev->pid, SIGPIPE);
}
found_proc = true;
break;
}
prev = p;
prev = p.get();
}
}
@ -350,7 +351,9 @@ static void handle_child_status(pid_t pid, int status) {
}
process_t::process_t()
: type(), // gets set later
: is_first_in_job(),
is_last_in_job(),
type(), // gets set later
internal_block_node(NODE_OFFSET_INVALID),
pid(0),
pipe_write_fd(0),
@ -358,8 +361,7 @@ process_t::process_t()
completed(0),
stopped(0),
status(0),
count_help_magic(0),
next(NULL)
count_help_magic(0)
#ifdef HAVE__PROC_SELF_STAT
,
last_time(),
@ -368,20 +370,17 @@ process_t::process_t()
{
}
process_t::~process_t() { delete this->next; }
job_t::job_t(job_id_t jobid, const io_chain_t &bio)
: block_io(bio), first_process(NULL), pgid(0), tmodes(), job_id(jobid), flags(0) {}
: block_io(bio), pgid(0), tmodes(), job_id(jobid), flags(0) {}
job_t::~job_t() {
delete first_process;
release_job_id(job_id);
}
/// Return all the IO redirections. Start with the block IO, then walk over the processes.
io_chain_t job_t::all_io_redirections() const {
io_chain_t result = this->block_io;
for (process_t *p = this->first_process; p != NULL; p = p->next) {
for (const process_ptr_t &p : this->processes) {
result.append(p->io_chain());
}
return result;
@ -557,7 +556,7 @@ int job_reap(bool allow_interactive) {
continue;
}
for (process_t *p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
int s;
if (!p->completed) continue;
@ -575,7 +574,7 @@ int job_reap(bool allow_interactive) {
}
// Handle signals other than SIGPIPE.
int proc_is_job = ((p == j->first_process) && (p->next == 0));
int proc_is_job = (p->is_first_in_job && p->is_last_in_job);
if (proc_is_job) job_set_flag(j, JOB_NOTIFIED, 1);
if (job_get_flag(j, JOB_SKIP_NOTIFICATION)) {
continue;
@ -884,9 +883,7 @@ void job_continue(job_t *j, bool cont) {
// Send the job a continue signal, if necessary.
if (cont) {
process_t *p;
for (p = j->first_process; p; p = p->next) p->stopped = 0;
for (process_ptr_t &p : j->processes) p->stopped = false;
if (job_get_flag(j, JOB_CONTROL)) {
if (killpg(j->pgid, SIGCONT)) {
@ -894,7 +891,7 @@ void job_continue(job_t *j, bool cont) {
return;
}
} else {
for (p = j->first_process; p; p = p->next) {
for (const process_ptr_t &p : j->processes) {
if (kill(p->pid, SIGCONT) < 0) {
wperror(L"kill (SIGCONT)");
return;
@ -951,8 +948,7 @@ void job_continue(job_t *j, bool cont) {
// order!
read_try(j);
process_t *p = j->first_process;
while (p->next) p = p->next;
const std::unique_ptr<process_t> &p = j->processes.back();
// Mark process status only if we are in the foreground and the last process in a pipe,
// and it is not a short circuited builtin.
@ -994,11 +990,9 @@ void proc_sanity_check() {
job_iterator_t jobs;
while ((j = jobs.next())) {
process_t *p;
if (!job_get_flag(j, JOB_CONSTRUCTED)) continue;
validate_pointer(j->first_process, _(L"Process list pointer"), 0);
// More than one foreground job?
if (job_get_flag(j, JOB_FOREGROUND) && !(job_is_stopped(j) || job_is_completed(j))) {
@ -1010,13 +1004,11 @@ void proc_sanity_check() {
fg_job = j;
}
p = j->first_process;
while (p) {
for (const process_ptr_t &p : j->processes) {
// Internal block nodes do not have argv - see issue #1545.
bool null_ok = (p->type == INTERNAL_BLOCK_NODE);
validate_pointer(p->get_argv(), _(L"Process argument list"), null_ok);
validate_pointer(p->argv0(), _(L"Process name"), null_ok);
validate_pointer(p->next, _(L"Process list pointer"), true);
if ((p->stopped & (~0x00000001)) != 0) {
debug(0, _(L"Job '%ls', process '%ls' has inconsistent state \'stopped\'=%d"),
@ -1030,7 +1022,6 @@ void proc_sanity_check() {
sanity_lose();
}
p = p->next;
}
}
}

18
src/proc.h
View file

@ -90,7 +90,10 @@ class process_t {
public:
process_t();
~process_t();
// Note whether we are the first and/or last in the job
bool is_first_in_job;
bool is_last_in_job;
/// Type of process. Can be one of \c EXTERNAL, \c INTERNAL_BUILTIN, \c INTERNAL_FUNCTION, \c
/// INTERNAL_EXEC.
@ -140,8 +143,6 @@ class process_t {
volatile int status;
/// Special flag to tell the evaluation function for count to print the help information.
int count_help_magic;
/// Next process in pipeline. We own this and we are responsible for deleting it.
process_t *next;
#ifdef HAVE__PROC_SELF_STAT
/// Last time of cpu time check.
struct timeval last_time;
@ -150,6 +151,9 @@ class process_t {
#endif
};
typedef std::unique_ptr<process_t> process_ptr_t;
typedef std::vector<process_ptr_t> process_list_t;
/// Constants for the flag variable in the job struct.
enum {
/// Whether the user has been told about stopped job.
@ -204,9 +208,9 @@ class job_t {
/// Sets the command.
void set_command(const wcstring &cmd) { command_str = cmd; }
/// A linked list of all the processes in this job. We are responsible for deleting this when we
/// are deallocated.
process_t *first_process;
/// All the processes in this job.
process_list_t processes;
/// Process group ID for the process group that this job is running in.
pid_t pgid;
/// The saved terminal modes of this job. This needs to be saved so that we can restore the
@ -342,7 +346,7 @@ void job_handle_signal(int signal, siginfo_t *info, void *con);
int job_signal(job_t *j, int signal);
/// Mark a process as failed to execute (and therefore completed).
void job_mark_process_as_failed(const job_t *job, process_t *p);
void job_mark_process_as_failed(job_t *job, const process_t *p);
#ifdef HAVE__PROC_SELF_STAT
/// Use the procfs filesystem to look up how many jiffies of cpu time was used by this process. This