diff --git a/src/autoload.cpp b/src/autoload.cpp index 7dc660a73..c91110db6 100644 --- a/src/autoload.cpp +++ b/src/autoload.cpp @@ -198,6 +198,12 @@ file_access_attempt_t access_file(const wcstring &path, int mode) { return result; } +void autoloader_t::perform_autoload(const wcstring &path) { + wcstring script_source = L"source " + escape_string(path, ESCAPE_ALL); + exec_subshell(script_source, parser_t::principal_parser(), + false /* do not apply exit status */); +} + autoload_t::autoload_t(wcstring env_var_name_var) : env_var_name(std::move(env_var_name_var)) {} void autoload_t::entry_was_evicted(wcstring key, autoload_function_t node) { diff --git a/src/autoload.h b/src/autoload.h index fd0a80875..df5ea1575 100644 --- a/src/autoload.h +++ b/src/autoload.h @@ -54,6 +54,11 @@ class autoloader_t { /// code; it is the caller's responsibility to load the file. maybe_t resolve_command(const wcstring &cmd, const environment_t &env); + /// Helper to actually perform an autoload. + /// This is a static function because it executes fish script, and so must be called without + /// holding any particular locks. + static void perform_autoload(const wcstring &path); + /// Mark that a command previously returned from path_to_autoload is finished autoloading. void mark_autoload_finished(const wcstring &cmd) { size_t amt = current_autoloading_.erase(cmd); diff --git a/src/function.cpp b/src/function.cpp index 1ab1d6c1a..32346e6eb 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -22,6 +22,7 @@ #include "common.h" #include "env.h" #include "event.h" +#include "exec.h" #include "fallback.h" // IWYU pragma: keep #include "function.h" #include "intern.h" @@ -31,65 +32,89 @@ #include "wutil.h" // IWYU pragma: keep class function_info_t { -public: - /// Immutable properties of the function. - std::shared_ptr props; - /// Function description. This may be changed after the function is created. - wcstring description; - /// File where this function was defined (intern'd string). - const wchar_t *const definition_file; - /// Mapping of all variables that were inherited from the function definition scope to their - /// values. - const std::map inherit_vars; - /// Flag for specifying that this function was automatically loaded. - const bool is_autoload; + public: + /// Immutable properties of the function. + std::shared_ptr props; + /// Function description. This may be changed after the function is created. + wcstring description; + /// File where this function was defined (intern'd string). + const wchar_t *const definition_file; + /// Mapping of all variables that were inherited from the function definition scope to their + /// values. + const std::map inherit_vars; + /// Flag for specifying that this function was automatically loaded. + const bool is_autoload; - /// Constructs relevant information from the function_data. - function_info_t(function_data_t data, const environment_t &vars, const wchar_t *filename, - bool autoload); + /// Constructs relevant information from the function_data. + function_info_t(function_data_t data, const environment_t &vars, const wchar_t *filename, + bool autoload); - /// Used by function_copy. - function_info_t(const function_info_t &data, const wchar_t *filename, bool autoload); + /// Used by function_copy. + function_info_t(const function_info_t &data, const wchar_t *filename, bool autoload); }; -/// Table containing all functions. -typedef std::unordered_map function_map_t; -static function_map_t loaded_functions; +/// Type wrapping up the set of all functions. +/// There's only one of these; it's managed by a lock. +struct function_set_t { + /// The map of all functions by name. + std::unordered_map funcs; -/// Functions that shouldn't be autoloaded (anymore). -static std::unordered_set function_tombstones; + /// Tombstones for functions that should no longer be autoloaded. + std::unordered_set autoload_tombstones; -/// Lock for functions. -static std::recursive_mutex functions_lock; + /// The autoloader for our functions. + autoloader_t autoloader{L"fish_function_path"}; -// Function autoloader -static autoload_t function_autoloader(L"fish_function_path"); + /// Remove a function. + /// \return true if successful, false if it doesn't exist. + bool remove(const wcstring &name); -/// Kludgy flag set by the load function in order to tell function_add that the function being -/// defined is autoloaded. There should be a better way to do this... -static bool is_autoload = false; + /// Get the info for a function, or nullptr if none. + const function_info_t *get_info(const wcstring &name) const { + auto iter = funcs.find(name); + return iter == funcs.end() ? nullptr : &iter->second; + } + + /// \return true if we should allow autoloading a given function. + bool allow_autoload(const wcstring &name) const; + + function_set_t() = default; +}; + +/// The big set of all functions. +static owning_lock function_set; + +bool function_set_t::allow_autoload(const wcstring &name) const { + // Prohibit autoloading if we have a non-autoload (explicit) function, or if the function is + // tombstoned. + const auto *info = get_info(name); + bool has_explicit_func = info && !info->is_autoload; + bool is_tombstoned = autoload_tombstones.count(name) > 0; + return !has_explicit_func && !is_tombstoned; +} /// Make sure that if the specified function is a dynamically loaded function, it has been fully /// loaded. -static int load(const wcstring &name) { +/// Note this executes fish script code. +static void try_autoload(const wcstring &name) { ASSERT_IS_MAIN_THREAD(); - scoped_rlock locker(functions_lock); - bool was_autoload = is_autoload; - int res; - - bool no_more_autoload = function_tombstones.count(name) > 0; - if (no_more_autoload) return 0; - - function_map_t::iterator iter = loaded_functions.find(name); - if (iter != loaded_functions.end() && !iter->second.is_autoload) { - // We have a non-autoload version already. - return 0; + maybe_t path_to_autoload; + // Note we can't autoload while holding the funcset lock. + // Lock around a local region. + { + auto funcset = function_set.acquire(); + if (funcset->allow_autoload(name)) { + const environment_t &vars = parser_t::principal_parser().vars(); + path_to_autoload = funcset->autoloader.resolve_command(name, vars); + } } - is_autoload = true; - res = function_autoloader.load(name, true); - is_autoload = was_autoload; - return res; + // Release the lock and perform any autoload, then reacquire the lock and clean up. + if (path_to_autoload) { + // Crucially, the lock is acquired *after* do_autoload_file_at_path(). + autoloader_t::perform_autoload(*path_to_autoload); + function_set.acquire()->autoloader.mark_autoload_finished(name); + } } /// Insert a list of all dynamically loaded functions into the specified list. @@ -151,21 +176,26 @@ function_info_t::function_info_t(const function_info_t &data, const wchar_t *fil is_autoload(autoload) {} void function_add(const function_data_t &data, const parser_t &parser) { - UNUSED(parser); ASSERT_IS_MAIN_THREAD(); + auto funcset = function_set.acquire(); - CHECK(!data.name.empty(), ); //!OCLINT(multiple unary operator) - scoped_rlock locker(functions_lock); + // Historical check. TODO: rationalize this. + if (data.name.empty()) { + return; + } // Remove the old function. - function_remove(data.name); + funcset->remove(data.name); + + // Check if this is a function that we are autoloading. + bool is_autoload = funcset->autoloader.autoload_in_progress(data.name); // Create and store a new function. const wchar_t *filename = reader_current_filename(); - - const function_map_t::value_type new_pair( - data.name, function_info_t(data, parser.vars(), filename, is_autoload)); - loaded_functions.insert(new_pair); + auto ins = funcset->funcs.emplace(data.name, + function_info_t(data, parser.vars(), filename, is_autoload)); + assert(ins.second && "Function should not already be present in the table"); + (void)ins; // Add event handlers. for (const event_description_t &ed : data.events) { @@ -175,129 +205,111 @@ void function_add(const function_data_t &data, const parser_t &parser) { std::shared_ptr function_get_properties(const wcstring &name) { if (parser_keywords_is_reserved(name)) return nullptr; - scoped_rlock locker(functions_lock); - auto where = loaded_functions.find(name); - if (where != loaded_functions.end()) { - return where->second.props; + auto funcset = function_set.acquire(); + if (const auto *info = funcset->get_info(name)) { + return info->props; } return nullptr; } int function_exists(const wcstring &cmd) { + ASSERT_IS_MAIN_THREAD(); if (parser_keywords_is_reserved(cmd)) return 0; - scoped_rlock locker(functions_lock); - load(cmd); - return loaded_functions.find(cmd) != loaded_functions.end(); + try_autoload(cmd); + auto funcset = function_set.acquire(); + return funcset->funcs.find(cmd) != funcset->funcs.end(); } void function_load(const wcstring &cmd) { + ASSERT_IS_MAIN_THREAD(); if (!parser_keywords_is_reserved(cmd)) { - scoped_rlock locker(functions_lock); - load(cmd); + try_autoload(cmd); } } int function_exists_no_autoload(const wcstring &cmd, const environment_t &vars) { if (parser_keywords_is_reserved(cmd)) return 0; - scoped_rlock locker(functions_lock); - return loaded_functions.find(cmd) != loaded_functions.end() || - function_autoloader.can_load(cmd, vars); + auto funcset = function_set.acquire(); + + // Check if we either have the function, or it could be autoloaded. + return funcset->get_info(cmd) || funcset->autoloader.can_autoload(cmd); } -static bool function_remove_ignore_autoload(const wcstring &name) { - // Note: the lock may be held at this point, but is recursive. - scoped_rlock locker(functions_lock); - - function_map_t::iterator iter = loaded_functions.find(name); - - // Not found. Not erasing. - if (iter == loaded_functions.end()) return false; - - // If we are removing an auto-loaded function, prevent it from being auto-reloaded. - if (iter->second.is_autoload) function_tombstones.insert(name); - - loaded_functions.erase(iter); - event_remove_function_handlers(name); - return true; +bool function_set_t::remove(const wcstring &name) { + size_t amt = funcs.erase(name); + if (amt > 0) { + event_remove_function_handlers(name); + } + return amt > 0; } void function_remove(const wcstring &name) { - if (function_remove_ignore_autoload(name)) function_autoloader.unload(name); -} - -/// Returns a function by name if it has been loaded, returns false otherwise. Does not autoload. -static const function_info_t *function_get(const wcstring &name) { - // The caller must lock the functions_lock before calling this; however our mutex is currently - // recursive, so trylock will never fail. We need a way to correctly check if a lock is locked - // (or better yet, make our lock non-recursive). - // ASSERT_IS_LOCKED(functions_lock); - function_map_t::iterator iter = loaded_functions.find(name); - if (iter == loaded_functions.end()) { - return NULL; + auto funcset = function_set.acquire(); + if (funcset->remove(name)) { + // Prevent re-autoloading this function. + funcset->autoload_tombstones.insert(name); } - return &iter->second; } bool function_get_definition(const wcstring &name, wcstring &out_definition) { - scoped_rlock locker(functions_lock); - const function_info_t *func = function_get(name); - if (func) { + const auto funcset = function_set.acquire(); + if (const function_info_t *func = funcset->get_info(name)) { auto props = func->props; if (props && props->parsed_source) { out_definition = props->body_node.get_source(props->parsed_source->src); } + return true; } - return func != NULL; + return false; } std::map function_get_inherit_vars(const wcstring &name) { - scoped_rlock locker(functions_lock); - const function_info_t *func = function_get(name); + const auto funcset = function_set.acquire(); + const function_info_t *func = funcset->get_info(name); return func ? func->inherit_vars : std::map(); } bool function_get_desc(const wcstring &name, wcstring &out_desc) { - // Empty length string goes to NULL. - scoped_rlock locker(functions_lock); - const function_info_t *func = function_get(name); + const auto funcset = function_set.acquire(); + const function_info_t *func = funcset->get_info(name); if (func && !func->description.empty()) { out_desc = _(func->description.c_str()); return true; } - return false; } void function_set_desc(const wcstring &name, const wcstring &desc) { - load(name); - scoped_rlock locker(functions_lock); - function_map_t::iterator iter = loaded_functions.find(name); - if (iter != loaded_functions.end()) { + ASSERT_IS_MAIN_THREAD(); + try_autoload(name); + auto funcset = function_set.acquire(); + auto iter = funcset->funcs.find(name); + if (iter != funcset->funcs.end()) { iter->second.description = desc; } } bool function_copy(const wcstring &name, const wcstring &new_name) { - bool result = false; - scoped_rlock locker(functions_lock); - function_map_t::const_iterator iter = loaded_functions.find(name); - if (iter != loaded_functions.end()) { - // This new instance of the function shouldn't be tied to the definition file of the - // original, so pass NULL filename, etc. - const function_map_t::value_type new_pair(new_name, - function_info_t(iter->second, NULL, false)); - loaded_functions.insert(new_pair); - result = true; + auto funcset = function_set.acquire(); + auto iter = funcset->funcs.find(name); + if (iter == funcset->funcs.end()) { + // No such function. + return false; } - return result; + + // This new instance of the function shouldn't be tied to the definition file of the + // original, so pass NULL filename, etc. + // Note this will NOT overwrite an existing function with the new name. + // TODO: rationalize if this behavior is desired. + funcset->funcs.emplace(new_name, function_info_t(iter->second, nullptr, false)); + return true; } wcstring_list_t function_get_names(int get_hidden) { std::unordered_set names; - scoped_rlock locker(functions_lock); + auto funcset = function_set.acquire(); autoload_names(names, get_hidden); - - for (const auto &func : loaded_functions) { + for (const auto &func : funcset->funcs) { const wcstring &name = func.first; // Maybe skip hidden. @@ -310,20 +322,20 @@ wcstring_list_t function_get_names(int get_hidden) { } const wchar_t *function_get_definition_file(const wcstring &name) { - scoped_rlock locker(functions_lock); - const function_info_t *func = function_get(name); + const auto funcset = function_set.acquire(); + const function_info_t *func = funcset->get_info(name); return func ? func->definition_file : NULL; } bool function_is_autoloaded(const wcstring &name) { - scoped_rlock locker(functions_lock); - const function_info_t *func = function_get(name); - return func->is_autoload; + const auto funcset = function_set.acquire(); + const function_info_t *func = funcset->get_info(name); + return func ? func->is_autoload : false; } int function_get_definition_lineno(const wcstring &name) { - scoped_rlock locker(functions_lock); - const function_info_t *func = function_get(name); + const auto funcset = function_set.acquire(); + const function_info_t *func = funcset->get_info(name); if (!func) return -1; // return one plus the number of newlines at offsets less than the start of our function's // statement (which includes the header). @@ -338,7 +350,22 @@ int function_get_definition_lineno(const wcstring &name) { return 1 + std::count(source.begin(), source.begin() + func_start, L'\n'); } -void function_invalidate_path() { function_autoloader.invalidate(); } +void function_invalidate_path() { + // Remove all autoloaded functions and update the autoload path. + // Note we don't want to risk removal during iteration; we expect this to be called + // infrequently. + auto funcset = function_set.acquire(); + wcstring_list_t autoloadees; + for (const auto &kv : funcset->funcs) { + if (kv.second.is_autoload) { + autoloadees.push_back(kv.first); + } + } + for (const wcstring &name : autoloadees) { + funcset->remove(name); + } + funcset->autoloader.clear(); +} // Setup the environment for the function. There are three components of the environment: // 1. argv