From 2c3214cabd733f1d94d2d00b448eb3a622953edb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 10 Feb 2019 13:43:02 -0800 Subject: [PATCH] Make $pipestatus thread safe and other misc cleanup --- src/env.cpp | 10 +++++----- src/event.cpp | 2 +- src/exec.cpp | 2 +- src/input.cpp | 2 +- src/parse_execution.cpp | 5 +++-- src/proc.cpp | 29 ++++++++++------------------- src/proc.h | 7 +++---- 7 files changed, 24 insertions(+), 33 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index 48b91be06..eecec7ff7 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -1392,13 +1392,13 @@ maybe_t env_stack_t::get(const wcstring &key, env_mode_flags_t mode) if (history) history->get_history(result); return env_var_t(L"history", result); } else if (key == L"pipestatus") { - const auto& js = proc_get_last_job_statuses(); + const auto js = proc_get_last_job_statuses(); wcstring_list_t result; - result.reserve(js->size()); - for (auto&& i : *js) { - result.emplace_back(to_string(i)); + result.reserve(js.size()); + for (int i : js) { + result.push_back(to_string(i)); } - return env_var_t(L"pipestatus", result); + return env_var_t(L"pipestatus", std::move(result)); } else if (key == L"status") { return env_var_t(L"status", to_string(proc_get_last_status())); } else if (key == L"umask") { diff --git a/src/event.cpp b/src/event.cpp index 63c4a8972..bdf002301 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -275,7 +275,7 @@ static void event_fire_internal(const event_t &event) { // non-interactive. proc_push_interactive(0); int prev_status = proc_get_last_status(); - const auto& saved_job_statuses = proc_get_last_job_statuses(); + auto saved_job_statuses = proc_get_last_job_statuses(); parser_t &parser = parser_t::principal_parser(); event_block_t *b = parser.push_block(event); diff --git a/src/exec.cpp b/src/exec.cpp index 217f416da..eec96702f 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1066,7 +1066,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, wcstrin ASSERT_IS_MAIN_THREAD(); bool prev_subshell = is_subshell; const int prev_status = proc_get_last_status(); - const auto& prev_job_statuses = proc_get_last_job_statuses(); + auto prev_job_statuses = proc_get_last_job_statuses(); bool split_output = false; const auto ifs = parser.vars().get(L"IFS"); diff --git a/src/input.cpp b/src/input.cpp index 24eec3f52..ca2c1ae46 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -367,7 +367,7 @@ static void input_mapping_execute(const input_mapping_t &m, bool allow_commands) // FIXME(snnw): if commands add stuff to input queue (e.g. commandline -f execute), we won't // see that until all other commands have also been run. int last_status = proc_get_last_status(); - const auto& last_job_statuses = proc_get_last_job_statuses(); + auto last_job_statuses = proc_get_last_job_statuses(); for (const wcstring &cmd : m.commands) { parser_t::principal_parser().eval(cmd, io_chain_t(), TOP); } diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index c4935df02..5db47a55a 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -717,8 +717,9 @@ parse_execution_result_t parse_execution_context_t::handle_command_not_found( } // Set the last proc status appropriately. - proc_set_last_status(err_code == ENOENT ? STATUS_CMD_UNKNOWN : STATUS_NOT_EXECUTABLE); - proc_set_last_job_statuses(proc_get_last_status()); + int status = err_code == ENOENT ? STATUS_CMD_UNKNOWN : STATUS_NOT_EXECUTABLE; + proc_set_last_status(status); + proc_set_last_job_statuses({status}); return parse_execution_errored; } diff --git a/src/proc.cpp b/src/proc.cpp index 4bc504426..9e444b852 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -54,7 +54,7 @@ static int last_status = 0; /// Statuses of last job's processes to exit - ensure we start off with one entry of 0. -static std::shared_ptr> last_job_statuses = std::make_shared>(1); +static owning_lock> last_job_statuses{std::vector(1u, 0)}; /// The signals that signify crashes to us. static const int crashsignals[] = {SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGSYS}; @@ -144,30 +144,21 @@ int proc_get_last_status() { return last_status; } void proc_set_last_job_statuses(const job_t &last_job) { ASSERT_IS_MAIN_THREAD(); - auto ljs = std::make_shared>(); - ljs->reserve(last_job.processes.size()); - for (auto &&p : last_job.processes) { - ljs->emplace_back(p->pid ? proc_format_status(p->status) : p->status); + std::vector ljs; + ljs.reserve(last_job.processes.size()); + for (const auto &p : last_job.processes) { + ljs.push_back(p->pid ? proc_format_status(p->status) : p->status); } - last_job_statuses = std::move(ljs); + proc_set_last_job_statuses(std::move(ljs)); } -void proc_set_last_job_statuses(std::shared_ptr> job_statuses) { +void proc_set_last_job_statuses(std::vector statuses) { ASSERT_IS_MAIN_THREAD(); - last_job_statuses = std::move(job_statuses); + auto vals = last_job_statuses.acquire(); + *vals = std::move(statuses); } -void proc_set_last_job_statuses(const int job_status) { - ASSERT_IS_MAIN_THREAD(); - auto ljs = std::make_shared>(1); - (*ljs)[0] = job_status; - last_job_statuses = std::move(ljs); -} - -std::shared_ptr> proc_get_last_job_statuses() { - ASSERT_IS_MAIN_THREAD(); - return last_job_statuses; -} +std::vector proc_get_last_job_statuses() { return *last_job_statuses.acquire(); } // Basic thread safe job IDs. The vector consumed_job_ids has a true value wherever the job ID // corresponding to that slot is in use. The job ID corresponding to slot 0 is 1. diff --git a/src/proc.h b/src/proc.h index a8adf73db..996b4ee60 100644 --- a/src/proc.h +++ b/src/proc.h @@ -418,11 +418,10 @@ int proc_get_last_status(); /// Sets the status of the last job's processes to exit from last_job. void proc_set_last_job_statuses(const job_t &last_job); -void proc_set_last_job_statuses(std::shared_ptr>); -void proc_set_last_job_statuses(const int); // for errors where pipe is unknown +void proc_set_last_job_statuses(std::vector statuses); -/// Returns the status of the last job's processes to exit. -std::shared_ptr> proc_get_last_job_statuses(); +/// Returns the statuses of the last job's processes to exit. +std::vector proc_get_last_job_statuses(); /// Notify the user about stopped or terminated jobs. Delete terminated jobs from the job list. ///