From 50f6b06251293d10a5dc85cfe9ea4f9f2eca41fe Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 19 Jun 2022 16:27:06 -0700 Subject: [PATCH] Replace a bunch of ASSERT_IS_MAIN_THREAD Switch these to a new function parser.assert_can_execute(), in preparation for allowing execution off of the main thread. --- src/common.cpp | 5 +---- src/complete.cpp | 1 - src/env.cpp | 1 - src/env_dispatch.cpp | 1 - src/exec.cpp | 4 ++-- src/function.cpp | 9 ++++----- src/global_safety.h | 2 ++ src/input.cpp | 13 ++++--------- src/input_common.cpp | 1 - src/parser.cpp | 4 +++- src/parser.h | 3 +++ src/proc.cpp | 6 +++--- src/reader.cpp | 1 + 13 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/common.cpp b/src/common.cpp index 75964dca6..83536ceae 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1729,10 +1729,7 @@ void setup_fork_guards() { [] { pthread_atfork(nullptr, nullptr, [] { is_forked_proc = true; }); }); } -void save_term_foreground_process_group() { - ASSERT_IS_MAIN_THREAD(); - initial_fg_process_group = tcgetpgrp(STDIN_FILENO); -} +void save_term_foreground_process_group() { initial_fg_process_group = tcgetpgrp(STDIN_FILENO); } void restore_term_foreground_process_group_for_exit() { // We wish to restore the tty to the initial owner. There's two ways this can go wrong: diff --git a/src/complete.cpp b/src/complete.cpp index 0f616478b..befd42ec8 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -532,7 +532,6 @@ void completer_t::complete_strings(const wcstring &wc_escaped, const description /// If command to complete is short enough, substitute the description with the whatis information /// for the executable. void completer_t::complete_cmd_desc(const wcstring &str) { - ASSERT_IS_MAIN_THREAD(); if (!ctx.parser) return; wcstring cmd; diff --git a/src/env.cpp b/src/env.cpp index 0dfb000ae..e2cd95485 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -1133,7 +1133,6 @@ maybe_t env_stack_impl_t::try_set_electric(const wcstring &key, const query /// Set a universal variable, inheriting as applicable from the given old variable. void env_stack_impl_t::set_universal(const wcstring &key, wcstring_list_t val, const query_t &query) { - ASSERT_IS_MAIN_THREAD(); auto oldvar = uvars()->get(key); // Resolve whether or not to export. bool exports = false; diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index d37770080..9462de61d 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -187,7 +187,6 @@ static void guess_emoji_width(const environment_t &vars) { /// React to modifying the given variable. void env_dispatch_var_change(const wcstring &key, env_stack_t &vars) { - ASSERT_IS_MAIN_THREAD(); // Do nothing if not yet fully initialized. if (!s_var_dispatch_table) return; diff --git a/src/exec.cpp b/src/exec.cpp index 77e2fd2b8..735eb69d3 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1182,7 +1182,7 @@ static void populate_subshell_output(wcstring_list_t *lst, const separated_buffe static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, const job_group_ref_t &job_group, wcstring_list_t *lst, bool *break_expand, bool apply_exit_status, bool is_subcmd) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); auto &ld = parser.libdata(); scoped_push is_subshell(&ld.is_subshell, true); @@ -1225,7 +1225,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, int exec_subshell_for_expand(const wcstring &cmd, parser_t &parser, const job_group_ref_t &job_group, wcstring_list_t &outputs) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); bool break_expand = false; int ret = exec_subshell_internal(cmd, parser, job_group, &outputs, &break_expand, true, true); // Only return an error code if we should break expansion. diff --git a/src/function.cpp b/src/function.cpp index db7203a48..5de0bdbdd 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -86,7 +86,7 @@ static std::shared_ptr copy_props(const function_properti /// loaded. /// Note this executes fish script code. bool function_load(const wcstring &name, parser_t &parser) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); maybe_t path_to_autoload; // Note we can't autoload while holding the funcset lock. // Lock around a local region. @@ -138,7 +138,6 @@ static void autoload_names(std::unordered_set &names, bool get_hidden) } void function_add(wcstring name, std::shared_ptr props) { - ASSERT_IS_MAIN_THREAD(); assert(props && "Null props"); auto funcset = function_set.acquire(); @@ -165,14 +164,14 @@ function_properties_ref_t function_get_props(const wcstring &name) { } function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); if (parser_keywords_is_reserved(name)) return nullptr; function_load(name, parser); return function_get_props(name); } bool function_exists(const wcstring &cmd, parser_t &parser) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); if (!valid_func_name(cmd)) return false; return function_get_props_autoload(cmd, parser) != nullptr; } @@ -217,7 +216,7 @@ static wcstring get_function_body_source(const function_properties_t &props) { } void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); function_load(name, parser); auto funcset = function_set.acquire(); auto iter = funcset->funcs.find(name); diff --git a/src/global_safety.h b/src/global_safety.h index 8667c722b..24b491912 100644 --- a/src/global_safety.h +++ b/src/global_safety.h @@ -22,6 +22,8 @@ class latch_t : noncopyable_t, nonmovable_t { T *operator->() { return value_; } const T *operator->() const { return value_; } + bool is_set() const { return value_ != nullptr; } + void operator=(std::unique_ptr value) { assert(value_ == nullptr && "Latch variable initialized multiple times"); assert(value != nullptr && "Latch variable initialized with null"); diff --git a/src/input.cpp b/src/input.cpp index e360ffa1e..237cf0274 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -275,15 +275,12 @@ void input_mapping_set_t::add(wcstring sequence, const wchar_t *command, const w input_mapping_set_t::add(std::move(sequence), &command, 1, mode, sets_mode, user); } -static relaxed_atomic_bool_t s_input_initialized{false}; - /// Set up arrays used by readch to detect escape sequences for special keys and perform related /// initializations for our input subsystem. void init_input() { ASSERT_IS_MAIN_THREAD(); - if (s_input_initialized) return; - s_input_initialized = true; + if (s_terminfo_mappings.is_set()) return; s_terminfo_mappings = create_input_terminfo(); auto input_mapping = input_mappings(); @@ -905,7 +902,7 @@ static std::vector create_input_terminfo() { } bool input_terminfo_get_sequence(const wcstring &name, wcstring *out_seq) { - assert(s_input_initialized); + assert(s_terminfo_mappings.is_set()); for (const terminfo_mapping_t &m : *s_terminfo_mappings) { if (name == m.name) { // Found the mapping. @@ -923,8 +920,7 @@ bool input_terminfo_get_sequence(const wcstring &name, wcstring *out_seq) { } bool input_terminfo_get_name(const wcstring &seq, wcstring *out_name) { - assert(s_input_initialized); - + assert(s_terminfo_mappings.is_set()); for (const terminfo_mapping_t &m : *s_terminfo_mappings) { if (m.seq && seq == str2wcstring(*m.seq)) { out_name->assign(m.name); @@ -936,8 +932,7 @@ bool input_terminfo_get_name(const wcstring &seq, wcstring *out_name) { } wcstring_list_t input_terminfo_get_names(bool skip_null) { - assert(s_input_initialized); - + assert(s_terminfo_mappings.is_set()); wcstring_list_t result; const auto &mappings = *s_terminfo_mappings; result.reserve(mappings.size()); diff --git a/src/input_common.cpp b/src/input_common.cpp index 03a777907..a98187b2b 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -152,7 +152,6 @@ maybe_t input_event_queue_t::try_pop() { } char_event_t input_event_queue_t::readch() { - ASSERT_IS_MAIN_THREAD(); wchar_t res{}; mbstate_t state = {}; for (;;) { diff --git a/src/parser.cpp b/src/parser.cpp index 7190f19ca..433bf0f34 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -54,10 +54,12 @@ parser_t::~parser_t() = default; const std::shared_ptr parser_t::principal{new parser_t()}; parser_t &parser_t::principal_parser() { - ASSERT_IS_MAIN_THREAD(); + principal->assert_can_execute(); return *principal; } +void parser_t::assert_can_execute() const { ASSERT_IS_MAIN_THREAD(); } + int parser_t::set_var_and_fire(const wcstring &key, env_mode_flags_t mode, wcstring_list_t vals) { int res = vars().set(key, mode, std::move(vals)); if (res == ENV_OK) { diff --git a/src/parser.h b/src/parser.h index 16d9a606f..f8ac84c36 100644 --- a/src/parser.h +++ b/src/parser.h @@ -303,6 +303,9 @@ class parser_t : public std::enable_shared_from_this { /// Get the "principal" parser, whatever that is. static parser_t &principal_parser(); + /// Assert that this parser is allowed to execute on the current thread. + void assert_can_execute() const; + /// Global event blocks. event_blockage_list_t global_event_blocks; diff --git a/src/proc.cpp b/src/proc.cpp index 846f666a7..ff0baa1e2 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -361,7 +361,7 @@ static void reap_disowned_pids() { /// \param block_ok if no reapable processes have exited, block until one is (or until we receive a /// signal). static void process_mark_finished_children(parser_t &parser, bool block_ok) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); // Get the exit and signal generations of all reapable processes. // The exit generation tells us if we have an exit; the signal generation allows for detecting @@ -647,7 +647,7 @@ static void save_wait_handle_for_completed_job(const shared_ptr &job, /// Remove completed jobs from the job list, printing status messages as appropriate. /// \return whether something was printed. static bool process_clean_after_marking(parser_t &parser, bool allow_interactive) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); // This function may fire an event handler, we do not want to call ourselves recursively (to // avoid infinite recursion). @@ -728,7 +728,7 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive } bool job_reap(parser_t &parser, bool allow_interactive) { - ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); // Early out for the common case that there are no jobs. if (parser.jobs().empty()) { return false; diff --git a/src/reader.cpp b/src/reader.cpp index be6ec7c6e..89e53bb32 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -2954,6 +2954,7 @@ uint64_t reader_status_count() { return status_count; } /// Read interactively. Read input from stdin while providing editing facilities. static int read_i(parser_t &parser) { ASSERT_IS_MAIN_THREAD(); + parser.assert_can_execute(); reader_config_t conf; conf.complete_ok = true; conf.highlight_ok = true;