diff --git a/src/env.cpp b/src/env.cpp index a5d9b8f75..e2c7c50fc 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -65,11 +65,12 @@ static acquired_lock uvars() { return s_universal_variables->acquire(); } +/// Set when a universal variable has been modified but not yet been written to disk via sync(). +static relaxed_atomic_bool_t s_uvars_locally_modified{false}; + /// Whether we were launched with no_config; in this case setting a uvar instead sets a global. static relaxed_atomic_bool_t s_uvar_scope_is_global{false}; -bool env_universal_barrier() { return env_stack_t::principal().universal_barrier(); } - namespace { struct electric_var_t { enum { @@ -430,7 +431,9 @@ void env_init(const struct config_paths_t *paths, bool do_uvars, bool default_pa // Set up universal variables using the default path. callback_data_list_t callbacks; uvars()->initialize(callbacks); - env_universal_callbacks(&vars, callbacks); + for (const callback_data_t &cb : callbacks) { + env_dispatch_var_change(cb.key, vars); + } // Do not import variables that have the same name and value as // an exported universal variable. See issues #5258 and #5348. @@ -1289,23 +1292,25 @@ mod_result_t env_stack_impl_t::remove(const wcstring &key, int mode) { return result; } -bool env_stack_t::universal_barrier() { - // Only perform universal barriers for the principal env stack. - // This means that changes from other fish processes will only be visible when the "main thread - // runs." - if (!is_principal()) return false; - - ASSERT_IS_MAIN_THREAD(); - if (s_uvar_scope_is_global) return false; +std::vector env_stack_t::universal_sync(bool always) { + if (s_uvar_scope_is_global) return {}; + if (!always && !s_uvars_locally_modified) return {}; + s_uvars_locally_modified = false; callback_data_list_t callbacks; bool changed = uvars()->sync(callbacks); if (changed) { universal_notifier_t::default_notifier().post_notification(); } - - env_universal_callbacks(this, callbacks); - return changed || !callbacks.empty(); + // React internally to changes to special variables like LANG, and populate on-variable events. + std::vector result; + for (const callback_data_t &cb : callbacks) { + env_dispatch_var_change(cb.key, *this); + event_t evt = + cb.is_erase() ? event_t::variable_erase(cb.key) : event_t::variable_set(cb.key); + result.push_back(std::move(evt)); + } + return result; } statuses_t env_stack_t::get_last_statuses() const { @@ -1368,9 +1373,9 @@ int env_stack_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t env_dispatch_var_change(key, *this); } } - // If the principal stack modified universal variables, then post a barrier. - if (ret.uvar_modified && is_principal()) { - universal_barrier(); + // Mark if we modified a uvar. + if (ret.uvar_modified) { + s_uvars_locally_modified = true; } return ret.status; } @@ -1393,8 +1398,8 @@ int env_stack_t::remove(const wcstring &key, int mode) { env_dispatch_var_change(key, *this); } } - if (ret.uvar_modified && is_principal()) { - universal_barrier(); + if (ret.uvar_modified) { + s_uvars_locally_modified = true; } return ret.status; } diff --git a/src/env.h b/src/env.h index 35caefdcb..cab6e5a3a 100644 --- a/src/env.h +++ b/src/env.h @@ -259,10 +259,6 @@ class env_stack_t final : public environment_t { /// Pop the variable stack. Used for implementing local variables for functions and for-loops. void pop(); - /// Synchronizes all universal variable changes: writes everything out, reads stuff in. - /// \return true if something changed, false otherwise. - bool universal_barrier(); - /// Returns an array containing all exported variables in a format suitable for execv. std::shared_ptr export_arr(); @@ -284,6 +280,12 @@ class env_stack_t final : public environment_t { /// Slightly optimized implementation. wcstring get_pwd_slash() const override; + /// Synchronizes universal variable changes. + /// If \p always is set, perform synchronization even if there's no pending changes from this + /// instance (that is, look for changes from other fish instances). + /// \return a list of events for changed variables. + std::vector universal_sync(bool always); + // Compatibility hack; access the "environment stack" from back when there was just one. static const std::shared_ptr &principal_ref(); static env_stack_t &principal() { return *principal_ref(); } @@ -297,10 +299,6 @@ bool get_use_posix_spawn(); extern bool term_has_xn; // does the terminal have the "eat_newline_glitch" -/// Synchronizes all universal variable changes: writes everything out, reads stuff in. -/// \return true if any value changed. -bool env_universal_barrier(); - /// Returns true if we think the terminal supports setting its title. bool term_supports_setting_title(); diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index f989dbe6c..d37770080 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -194,19 +194,6 @@ void env_dispatch_var_change(const wcstring &key, env_stack_t &vars) { s_var_dispatch_table->dispatch(key, vars); } -// Trigger events due to a universal variable changing. -void env_universal_callbacks(env_stack_t *stack, const callback_data_list_t &callbacks) { - for (const callback_data_t &cb : callbacks) { - env_dispatch_var_change(cb.key, *stack); - - // TODO: eliminate this principal_parser. Need to rationalize how multiple threads work - // here. - event_t evt = - cb.is_erase() ? event_t::variable_erase(cb.key) : event_t::variable_set(cb.key); - event_fire(parser_t::principal_parser(), evt); - } -} - static void handle_fish_term_change(const env_stack_t &vars) { update_fish_color_support(vars); reader_schedule_prompt_repaint(); diff --git a/src/env_dispatch.h b/src/env_dispatch.h index 0102a3a85..c469e5d9b 100644 --- a/src/env_dispatch.h +++ b/src/env_dispatch.h @@ -9,13 +9,14 @@ #include "common.h" #include "env_universal_common.h" -/// Initialize variable dispatch. class environment_t; +class env_stack_t; +class parser_t; + +/// Initialize variable dispatch. void env_dispatch_init(const environment_t &vars); -class env_stack_t; +/// React to changes in variables like LANG which require running some code. void env_dispatch_var_change(const wcstring &key, env_stack_t &vars); -void env_universal_callbacks(env_stack_t *stack, const callback_data_list_t &callbacks); - #endif diff --git a/src/fish.cpp b/src/fish.cpp index 939016c11..80b2db71c 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -510,6 +510,7 @@ int main(int argc, char **argv) { reader_init(); parser_t &parser = parser_t::principal_parser(); + parser.set_syncs_uvars(!opts.no_config); if (!opts.no_exec && !opts.no_config) { read_init(parser, paths); diff --git a/src/input.cpp b/src/input.cpp index 4ead777f0..dc4825487 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -351,6 +351,10 @@ void inputter_t::select_interrupted() /* override */ { this->push_front(char_event_t{char_event_type_t::check_exit}); } +void inputter_t::uvar_change_notified() /* override */ { + this->parser_->sync_uvars_and_fire(true /* always */); +} + void inputter_t::function_push_arg(wchar_t arg) { input_function_args_.push_back(arg); } wchar_t inputter_t::function_pop_arg() { diff --git a/src/input.h b/src/input.h index f3d292d05..c2343baae 100644 --- a/src/input.h +++ b/src/input.h @@ -60,6 +60,9 @@ class inputter_t final : private input_event_queue_t { // Called when select() is interrupted by a signal. void select_interrupted() override; + // Called when we are notified of a uvar change. + void uvar_change_notified() override; + void function_push_arg(wchar_t arg); void function_push_args(readline_cmd_t code); void mapping_execute(const input_mapping_t &m, const command_handler_t &command_handler); diff --git a/src/input_common.cpp b/src/input_common.cpp index 7d5b5ece6..03a777907 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -180,7 +180,7 @@ char_event_t input_event_queue_t::readch() { break; case readb_uvar_notified: - env_universal_barrier(); + this->uvar_change_notified(); break; case readb_ioport_notified: @@ -266,4 +266,5 @@ void input_event_queue_t::promote_interruptions_to_front() { void input_event_queue_t::prepare_to_select() {} void input_event_queue_t::select_interrupted() {} +void input_event_queue_t::uvar_change_notified() {} input_event_queue_t::~input_event_queue_t() = default; diff --git a/src/input_common.h b/src/input_common.h index c02621492..da03e7ca8 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -225,6 +225,10 @@ class input_event_queue_t { /// Override point for when when select() is interrupted by a signal. The default does nothing. virtual void select_interrupted(); + /// Override point for when when select() is interrupted by the universal variable notifier. + /// The default does nothing. + virtual void uvar_change_notified(); + virtual ~input_event_queue_t(); private: diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 671c2998b..5fff7f486 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -1383,10 +1383,8 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo } // Update universal variables on external commands. - // TODO: justify this, why not on every command? - if (job->has_external_proc()) { - parser->vars().universal_barrier(); - } + // We only incorporate external changes if we had an external proc, for hysterical raisins. + parser->sync_uvars_and_fire(job->has_external_proc() /* always */); // If the job got a SIGINT or SIGQUIT, then we're going to start unwinding. if (!cancel_signal) cancel_signal = job->group->get_cancel_signal(); diff --git a/src/parser.cpp b/src/parser.cpp index 8c1f75239..7190f19ca 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -72,6 +72,15 @@ int parser_t::set_var_and_fire(const wcstring &key, env_mode_flags_t mode, wcstr return set_var_and_fire(key, mode, std::move(vals)); } +void parser_t::sync_uvars_and_fire(bool always) { + if (this->syncs_uvars_) { + auto evts = this->vars().universal_sync(always); + for (const auto &evt : evts) { + event_fire(*this, evt); + } + } +} + block_t *parser_t::push_block(block_t &&block) { block.src_lineno = parser_t::get_lineno(); block.src_filename = parser_t::current_filename(); diff --git a/src/parser.h b/src/parser.h index 85ee14c68..16d9a606f 100644 --- a/src/parser.h +++ b/src/parser.h @@ -259,13 +259,20 @@ class parser_t : public std::enable_shared_from_this { /// This is in "reverse" order: the topmost block is at the front. This enables iteration from /// top down using range-based for loops. std::deque block_list; + /// The 'depth' of the fish call stack. int eval_level = -1; + /// Set of variables for the parser. const std::shared_ptr variables; + /// Miscellaneous library data. library_data_t library_data{}; + /// If set, we synchronize universal variables after external commands, + /// including sending on-variable change events. + bool syncs_uvars_{false}; + /// List of profile items. /// This must be a deque because we return pointers to them to callers, /// who may hold them across blocks (which would cause reallocations internal @@ -381,6 +388,11 @@ class parser_t : public std::enable_shared_from_this { int set_var_and_fire(const wcstring &key, env_mode_flags_t mode, wcstring val); int set_var_and_fire(const wcstring &key, env_mode_flags_t mode, wcstring_list_t vals); + /// Update any universal variables and send event handlers. + /// If \p always is set, then do it even if we have no pending changes (that is, look for + /// changes from other fish instances); otherwise only sync if this instance has changed uvars. + void sync_uvars_and_fire(bool always = false); + /// Pushes a new block. Returns a pointer to the block, stored in the parser. The pointer is /// valid until the call to pop_block(). block_t *push_block(block_t &&b); @@ -429,6 +441,9 @@ class parser_t : public std::enable_shared_from_this { /// \return whether the number of functions in the stack exceeds our stack depth limit. bool function_stack_is_overflowing() const; + /// Mark whether we should sync universal variables. + void set_syncs_uvars(bool flag) { syncs_uvars_ = flag; } + /// \return a shared pointer reference to this parser. std::shared_ptr shared();