From 2b11d1060ccf8f7c2a255c64b991f6f6788843ae Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 15 May 2014 10:49:06 +0800 Subject: [PATCH] Make initialize_synchronizes_via_fishd default to false. Add some logging support to universal variables. --- env_universal.cpp | 3 ++ env_universal_common.cpp | 90 +++++++++++++++++++++++++--------------- env_universal_common.h | 9 ++++ 3 files changed, 69 insertions(+), 33 deletions(-) diff --git a/env_universal.cpp b/env_universal.cpp index 69b1f5459..eb699e48e 100644 --- a/env_universal.cpp +++ b/env_universal.cpp @@ -269,6 +269,8 @@ static void env_universal_remove_all() */ static void reconnect() { + assert(synchronizes_via_fishd()); + if (get_socket_count >= RECONNECT_COUNT) return; @@ -388,6 +390,7 @@ bool env_universal_get_export(const wcstring &name) void env_universal_barrier() { ASSERT_IS_MAIN_THREAD(); + UNIVERSAL_LOG("BARRIER"); message_t *msg; fd_set fds; diff --git a/env_universal_common.cpp b/env_universal_common.cpp index 1a6a95d0d..024f116f9 100644 --- a/env_universal_common.cpp +++ b/env_universal_common.cpp @@ -276,6 +276,7 @@ void env_universal_common_set(const wchar_t *key, const wchar_t *val, bool expor void env_universal_common_sync() { + assert(! synchronizes_via_fishd()); callback_data_list_t callbacks; bool changed = default_universal_vars().sync(&callbacks); if (changed) @@ -693,7 +694,11 @@ void env_universal_t::load_from_fd(int fd, callback_data_list_t *callbacks) assert(fd >= 0); /* Get the dev / inode */ const file_id_t current_file = file_id_for_fd(fd); - if (current_file != last_read_file) + if (current_file == last_read_file) + { + UNIVERSAL_LOG("Sync elided based on fstat()"); + } + else { /* Unmodified values are sourced from the file. Since we are about to read a different file, erase them */ this->erase_unmodified_values(); @@ -711,6 +716,7 @@ bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t /* Check to see if the file is unchanged. We do this again in load_from_fd, but this avoids opening the file unnecessarily. */ if (last_read_file != kInvalidFileID && file_id_for_path(path) == last_read_file) { + UNIVERSAL_LOG("Sync elided based on fast stat()"); return true; } @@ -719,6 +725,7 @@ bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t int fd = wopen_cloexec(path, O_RDONLY); if (fd >= 0) { + UNIVERSAL_LOG("Reading from file"); this->load_from_fd(fd, callbacks); close(fd); result = true; @@ -985,43 +992,50 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) #endif const wcstring directory = wdirname(vars_path); - bool success = false; + bool success = true; int vars_fd = -1; int private_fd = -1; wcstring private_file_path; - do + + UNIVERSAL_LOG("Performing full sync"); + + /* Open the file */ + if (success) + { + success = this->open_and_acquire_lock(vars_path, &vars_fd); + } + + /* Read from it */ + if (success) { - /* Open the file */ - if (! this->open_and_acquire_lock(vars_path, &vars_fd)) - { - break; - } - - /* Read from it */ assert(vars_fd >= 0); this->load_from_fd(vars_fd, callbacks); - - /* Open adjacent temporary file */ - if (! this->open_temporary_file(directory, &private_file_path, &private_fd)) - { - break; - } - - /* Write to it */ + } + + /* Open adjacent temporary file */ + if (success) + { + success = this->open_temporary_file(directory, &private_file_path, &private_fd); + } + + /* Write to it */ + if (success) + { assert(private_fd >= 0); this->write_to_fd(private_fd); - + } + + if (success) + { /* Apply new file */ - if (! this->move_new_vars_file_into_place(private_file_path, vars_path)) - { - break; - } - + success = this->move_new_vars_file_into_place(private_file_path, vars_path); + } + + if (success) + { /* Since we moved the new file into place, clear the path so we don't try to unlink it */ - private_file_path.clear(); - success = true; - break; - } while (false); + private_file_path.clear(); + } /* Clean up */ if (vars_fd >= 0) @@ -1907,7 +1921,7 @@ class universal_notifier_null_t : public universal_notifier_t static universal_notifier_t::notifier_strategy_t fetch_default_strategy_from_environment() { - if (! synchronizes_via_fishd()) + if (synchronizes_via_fishd()) { return universal_notifier_t::strategy_null; } @@ -2036,13 +2050,13 @@ bool universal_notifier_t::notification_fd_became_readable(int fd) return false; } -static bool initialize_synchronizes_via_fishd() +static bool bool_from_env_var(const char *name, bool default_value) { - const char *tmp = getenv("fish_use_fishd"); - return tmp != NULL && from_string(tmp); + const char *var = getenv(name); + return var ? from_string(var) : default_value; } -bool synchronizes_via_fishd() +static bool initialize_synchronizes_via_fishd() { if (program_name && ! wcscmp(program_name, L"fishd")) { @@ -2050,8 +2064,18 @@ bool synchronizes_via_fishd() return true; } + return bool_from_env_var(UNIVERSAL_USE_FISHD, true); +} + +bool synchronizes_via_fishd() +{ /* Note that in general we can't change this once it's been set, so we only load it once */ static bool result = initialize_synchronizes_via_fishd(); return result; } +bool universal_log_enabled() +{ + return bool_from_env_var(UNIVERSAL_LOGGING_ENV_NAME, false); +} + diff --git a/env_universal_common.h b/env_universal_common.h index c4eda46bf..c18f7e979 100644 --- a/env_universal_common.h +++ b/env_universal_common.h @@ -349,7 +349,16 @@ bool get_hostname_identifier(std::string *result); /* Temporary */ bool synchronizes_via_fishd(); +bool universal_log_enabled(); +#define UNIVERSAL_LOG(x) if (universal_log_enabled()) fprintf(stderr, "UNIVERSAL LOG: %s\n", x) + /* Environment variable for requesting a particular universal notifier. See fetch_default_strategy_from_environment for names. */ #define UNIVERSAL_NOTIFIER_ENV_NAME "fish_universal_notifier" +/* Environment variable for enabling universal variable logging (to stderr) */ +#define UNIVERSAL_LOGGING_ENV_NAME "fish_universal_log" + +/* Environment variable for enabling fishd */ +#define UNIVERSAL_USE_FISHD "fish_use_fishd" + #endif