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

Switch job handling to use shared pointers instead of raw pointers

Browse source 
Clarifies memory management around allocation of job_ts
This commit is contained in:
ridiculousfish 2017-01-26 14:47:32 -08:00
parent 1d9cc12984
commit 14fb38f952
8 changed files with 58 additions and 67 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/builtin.cpp
View file

@ -1681,7 +1681,7 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
event_t e(EVENT_ANY);
if ((opt == 'j') && (wcscasecmp(w.woptarg, L"caller") == 0)) {
int job_id = -1;
job_id_t job_id = -1;
if (is_subshell) {
size_t block_idx = 0;
@ -2861,10 +2861,6 @@ static int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **arg
return res;
}
/// Make the specified job the first job of the job list. Moving jobs around in the list makes the
/// list reflect the order in which the jobs were used.
static void make_first(job_t *j) { job_promote(j); }
/// Builtin for putting a job in the foreground.
static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
job_t *j = NULL;
@ -2941,7 +2937,7 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
if (!ft.empty()) env_set(L"_", ft.c_str(), ENV_EXPORT);
reader_write_title(j->command());
make_first(j);
job_promote(j);
job_set_flag(j, JOB_FOREGROUND, 1);
job_continue(j, job_is_stopped(j));
@ -2964,7 +2960,7 @@ static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j, const w
streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id,
j->command_wcstr());
make_first(j);
job_promote(j);
job_set_flag(j, JOB_FOREGROUND, 0);
job_continue(j, job_is_stopped(j));
return STATUS_BUILTIN_OK;

13
src/expand.cpp
View file

@ -435,7 +435,6 @@ bool process_iterator_t::next_process(wcstring *out_str, pid_t *out_pid) {
static bool find_job(const wchar_t *proc, expand_flags_t flags, std::vector<completion_t> *completions) {
ASSERT_IS_MAIN_THREAD();
const job_t *j;
bool found = false;
// If we are not doing tab completion, we first check for the single '%' character, because an
// empty string will pass the numeric check below. But if we are doing tab completion, we want
@ -444,7 +443,7 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, std::vector<comp
if (wcslen(proc) == 0 && !(flags & EXPAND_FOR_COMPLETIONS)) {
// This is an empty job expansion: '%'. It expands to the last job backgrounded.
job_iterator_t jobs;
while ((j = jobs.next())) {
while (const job_t *j = jobs.next()) {
if (!j->command_is_empty()) {
append_completion(completions, to_string<long>(j->pgid));
break;
@ -457,7 +456,7 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, std::vector<comp
// This is a numeric job string, like '%2'.
if (flags & EXPAND_FOR_COMPLETIONS) {
job_iterator_t jobs;
while ((j = jobs.next())) {
while (const job_t *j = jobs.next()) {
wchar_t jid[16];
if (j->command_is_empty()) continue;
@ -471,8 +470,8 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, std::vector<comp
} else {
int jid = fish_wcstoi(proc);
if (!errno && jid > 0) {
j = job_get(jid);
if ((j != 0) && (j->command_wcstr() != 0) && (!j->command_is_empty())) {
const job_t *j = job_get(jid);
if (j && !j->command_is_empty()) {
append_completion(completions, to_string<long>(j->pgid));
}
}
@ -487,7 +486,7 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, std::vector<comp
}
job_iterator_t jobs;
while ((j = jobs.next())) {
while (const job_t *j = jobs.next()) {
if (j->command_is_empty()) continue;
size_t offset;
@ -507,7 +506,7 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, std::vector<comp
}
jobs.reset();
while ((j = jobs.next())) {
while (const job_t *j = jobs.next()) {
if (j->command_is_empty()) continue;
for (const process_ptr_t &p : j->processes) {
if (p->actual_cmd.empty()) continue;

28
src/parse_execution.cpp
View file

@ -1265,34 +1265,32 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
return result;
}
job_t *j = new job_t(acquire_job_id(), block_io);
j->tmodes = tmodes;
job_set_flag(j, JOB_CONTROL,
shared_ptr<job_t> job = std::make_shared<job_t>(acquire_job_id(), block_io);
job->tmodes = tmodes;
job_set_flag(job.get(), JOB_CONTROL,
(job_control_mode == JOB_CONTROL_ALL) ||
((job_control_mode == JOB_CONTROL_INTERACTIVE) && shell_is_interactive()));
job_set_flag(j, JOB_FOREGROUND, !tree.job_should_be_backgrounded(job_node));
job_set_flag(job.get(), JOB_FOREGROUND, !tree.job_should_be_backgrounded(job_node));
job_set_flag(j, JOB_TERMINAL, job_get_flag(j, JOB_CONTROL) && !is_subshell && !is_event);
job_set_flag(job.get(), JOB_TERMINAL, job_get_flag(job.get(), JOB_CONTROL) && !is_subshell && !is_event);
job_set_flag(j, JOB_SKIP_NOTIFICATION,
job_set_flag(job.get(), JOB_SKIP_NOTIFICATION,
is_subshell || is_block || is_event || !shell_is_interactive());
// Tell the current block what its job is. This has to happen before we populate it (#1394).
parser->current_block()->job = j;
parser->current_block()->job = job;
// Populate the job. This may fail for reasons like command_not_found. If this fails, an error
// will have been printed.
parse_execution_result_t pop_result =
this->populate_job_from_job_node(j, job_node, associated_block);
this->populate_job_from_job_node(job.get(), job_node, associated_block);
// Clean up the job on failure or cancellation.
bool populated_job = (pop_result == parse_execution_success);
if (!populated_job || this->should_cancel_execution(associated_block)) {
assert(parser->current_block()->job == j);
assert(parser->current_block()->job == job);
parser->current_block()->job = NULL;
delete j;
j = NULL;
populated_job = false;
}
@ -1303,11 +1301,11 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
if (populated_job) {
// Success. Give the job to the parser - it will clean it up.
parser->job_add(j);
parser->job_add(job);
// Check to see if this contained any external commands.
bool job_contained_external_command = false;
for (const auto &proc : j->processes) {
for (const auto &proc : job->processes) {
if (proc->type == EXTERNAL) {
job_contained_external_command = true;
break;
@ -1315,7 +1313,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
}
// Actually execute the job.
exec_job(*this->parser, j);
exec_job(*this->parser, job.get());
// Only external commands require a new fishd barrier.
if (job_contained_external_command) {
@ -1328,7 +1326,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
profile_item->level = eval_level;
profile_item->parse = (int)(parse_time - start_time);
profile_item->exec = (int)(exec_time - parse_time);
profile_item->cmd = j ? j->command() : wcstring();
profile_item->cmd = job ? job->command() : wcstring();
profile_item->skipped = !populated_job;
}

26
src/parser.cpp
View file

@ -523,17 +523,18 @@ wcstring parser_t::current_line() {
return line_info;
}
void parser_t::job_add(job_t *job) {
void parser_t::job_add(shared_ptr<job_t> job) {
assert(job != NULL);
assert(! job->processes.empty());
this->my_job_list.push_front(job);
this->my_job_list.push_front(std::move(job));
}
bool parser_t::job_remove(job_t *job) {
job_list_t::iterator iter = std::find(my_job_list.begin(), my_job_list.end(), job);
if (iter != my_job_list.end()) {
my_job_list.erase(iter);
return true;
for (auto iter = my_job_list.begin(); iter != my_job_list.end(); ++iter) {
if (iter->get() == job) {
my_job_list.erase(iter);
return true;
}
}
debug(1, _(L"Job inconsistency"));
@ -542,7 +543,12 @@ bool parser_t::job_remove(job_t *job) {
}
void parser_t::job_promote(job_t *job) {
job_list_t::iterator loc = std::find(my_job_list.begin(), my_job_list.end(), job);
job_list_t::iterator loc;
for (loc = my_job_list.begin(); loc != my_job_list.end(); ++loc) {
if (loc->get() == job) {
break;
}
}
assert(loc != my_job_list.end());
// Move the job to the beginning.
@ -550,10 +556,8 @@ void parser_t::job_promote(job_t *job) {
}
job_t *parser_t::job_get(job_id_t id) {
job_iterator_t jobs(my_job_list);
job_t *job;
while ((job = jobs.next())) {
if (id <= 0 || job->job_id == id) return job;
for (const auto &job : my_job_list) {
if (id <= 0 || job->job_id == id) return job.get();
}
return NULL;
}

6
src/parser.h
View file

@ -78,7 +78,7 @@ struct block_t {
/// Status for the current loop block. Can be any of the values from the loop_status enum.
enum loop_status_t loop_status;
/// The job that is currently evaluated in the specified block.
job_t *job;
shared_ptr<job_t> job;
/// Name of file that created this block. This string is intern'd.
const wchar_t *src_filename;
/// Line number where this block was created.
@ -207,7 +207,7 @@ class parser_t {
parser_t &operator=(const parser_t &);
/// Adds a job to the beginning of the job list.
void job_add(job_t *job);
void job_add(shared_ptr<job_t> job);
/// Returns the name of the currently evaluated function if we are currently evaluating a
/// function, null otherwise. This is tested by moving down the block-scope-stack, checking
@ -310,7 +310,7 @@ class parser_t {
void job_promote(job_t *job);
/// Return the job with the specified job id. If id is 0 or less, return the last job used.
job_t *job_get(int job_id);
job_t *job_get(job_id_t job_id);
/// Returns the job with the given pid.
job_t *job_get_from_pid(int pid);

22
src/proc.cpp
View file

@ -138,18 +138,12 @@ void job_promote(job_t *job) {
parser_t::principal_parser().job_promote(job);
}
/// Remove job from the job list and free all memory associated with it.
void job_free(job_t *j) {
job_remove(j);
delete j;
}
void proc_destroy() {
job_list_t &jobs = parser_t::principal_parser().job_list();
while (!jobs.empty()) {
job_t *job = jobs.front();
job_t *job = jobs.front().get();
debug(2, L"freeing leaked job %ls", job->command_wcstr());
job_free(job);
job_remove(job);
}
}
@ -237,7 +231,7 @@ bool job_is_completed(const job_t *j) {
return result;
}
void job_set_flag(job_t *j, unsigned int flag, int set) {
void job_set_flag(job_t *j, job_flag_t flag, bool set) {
if (set) {
j->flags |= flag;
} else {
@ -245,7 +239,9 @@ void job_set_flag(job_t *j, unsigned int flag, int set) {
}
}
int job_get_flag(const job_t *j, unsigned int flag) { return static_cast<bool>(j->flags & flag); }
bool job_get_flag(const job_t *j, job_flag_t flag) {
return !! (j->flags & flag);
}
int job_signal(job_t *j, int signal) {
pid_t my_pid = getpid();
@ -630,7 +626,7 @@ int job_reap(bool allow_interactive) {
proc_fire_event(L"JOB_EXIT", EVENT_EXIT, -j->pgid, 0);
proc_fire_event(L"JOB_EXIT", EVENT_JOB_ID, j->job_id, 0);
job_free(j);
job_remove(j);
} else if (job_is_stopped(j) && !job_get_flag(j, JOB_NOTIFIED)) {
// Notify the user about newly stopped jobs.
if (!job_get_flag(j, JOB_SKIP_NOTIFICATION)) {
@ -983,10 +979,10 @@ int proc_format_status(int status) {
}
void proc_sanity_check() {
job_t *fg_job = NULL;
const job_t *fg_job = NULL;
job_iterator_t jobs;
while (job_t *j = jobs.next()) {
while (const job_t *j = jobs.next()) {
if (!job_get_flag(j, JOB_CONSTRUCTED)) continue;

18
src/proc.h
View file

@ -155,7 +155,7 @@ 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 {
enum job_flag_t {
/// Whether the user has been told about stopped job.
JOB_NOTIFIED = 1 << 0,
/// Whether this job is in the foreground.
@ -189,8 +189,8 @@ class job_t {
const io_chain_t block_io;
// No copying.
job_t(const job_t &rhs);
void operator=(const job_t &);
job_t(const job_t &rhs) = delete;
void operator=(const job_t &) = delete;
public:
job_t(job_id_t jobid, const io_chain_t &bio);
@ -249,7 +249,8 @@ extern int is_login;
/// Whether we are running an event handler.
extern int is_event;
typedef std::list<job_t *> job_list_t;
// List of jobs. We sometimes mutate this while iterating - hence it must be a list, not a vector
typedef std::list<shared_ptr<job_t>> job_list_t;
bool job_list_is_empty(void);
@ -264,7 +265,7 @@ class job_iterator_t {
job_t *next() {
job_t *job = NULL;
if (current != end) {
job = *current;
job = current->get();
++current;
}
return job;
@ -298,10 +299,10 @@ extern int job_control_mode;
extern int no_exec;
/// Add the specified flag to the bitset of flags for the specified job.
void job_set_flag(job_t *j, unsigned int flag, int set);
void job_set_flag(job_t *j, job_flag_t flag, bool set);
/// Returns one if the specified flag is set in the specified job, 0 otherwise.
int job_get_flag(const job_t *j, unsigned int flag);
bool job_get_flag(const job_t *j, job_flag_t flag);
/// Sets the status of the last process to exit.
void proc_set_last_status(int s);
@ -309,9 +310,6 @@ void proc_set_last_status(int s);
/// Returns the status of the last process to exit.
int proc_get_last_status();
/// Remove the specified job.
void job_free(job_t *j);
/// Promotes a job to the front of the job list.
void job_promote(job_t *job);

2
src/reader.cpp
View file

@ -2184,7 +2184,7 @@ static void handle_end_loop() {
}
bool bg_jobs = false;
while (job_t *j = jobs.next()) {
while (const job_t *j = jobs.next()) {
if (!job_is_completed(j)) {
bg_jobs = true;
break;