diff --git a/configure.ac b/configure.ac index f0c4d8b5b..b5c165527 100644 --- a/configure.ac +++ b/configure.ac @@ -525,6 +525,7 @@ AC_CHECK_FUNCS( wcsdup wcsndup wcslen wcscasecmp wcsncasecmp fwprintf ) AC_CHECK_FUNCS( futimes wcwidth wcswidth wcstok fputwc fgetwc ) AC_CHECK_FUNCS( wcstol wcslcat wcslcpy lrand48_r killpg mkostemp ) AC_CHECK_FUNCS( backtrace backtrace_symbols sysconf getifaddrs ) +AC_CHECK_FUNCS( futimens clock_gettime ) if test x$local_gettext != xno; then AC_CHECK_FUNCS( gettext dcgettext ) diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index dabac1fab..45333a715 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -808,6 +808,7 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) if (modified.empty()) { this->load_from_path(vars_path, callbacks); + UNIVERSAL_LOG("No modifications"); return false; } @@ -823,6 +824,7 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) if (success) { success = this->open_and_acquire_lock(vars_path, &vars_fd); + if (! success) UNIVERSAL_LOG("open_and_acquire_lock() failed"); } /* Read from it */ @@ -831,11 +833,13 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) assert(vars_fd >= 0); this->load_from_fd(vars_fd, callbacks); } + /* Open adjacent temporary file */ if (success) { success = this->open_temporary_file(directory, &private_file_path, &private_fd); + if (! success) UNIVERSAL_LOG("open_temporary_file() failed"); } /* Write to it */ @@ -843,6 +847,7 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) { assert(private_fd >= 0); success = this->write_to_fd(private_fd, private_file_path); + if (! success) UNIVERSAL_LOG("write_to_fd() failed"); } if (success) @@ -854,9 +859,26 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) fchown(private_fd, sbuf.st_uid, sbuf.st_gid); fchmod(private_fd, sbuf.st_mode); } - + + /* Linux by default stores the mtime with low precision, low enough that updates that occur in quick succession may + result in the same mtime (even the nanoseconds field). So manually set the mtime of the new file to a high-precision + clock. Note that this is only necessary because Linux aggressively reuses inodes, causing the ABA problem; on other + platforms we tend to notice the file has changed due to a different inode (or file size!) + + It's probably worth finding a simpler solution to this. The tests ran into this, but it's unlikely to affect users. + */ +#if HAVE_CLOCK_GETTIME && HAVE_FUTIMENS + struct timespec times[2] = {}; + times[0].tv_nsec = UTIME_OMIT; // don't change ctime + if (0 == clock_gettime(CLOCK_REALTIME, ×[1])) + { + futimens(private_fd, times); + } +#endif + /* Apply new file */ success = this->move_new_vars_file_into_place(private_file_path, vars_path); + if (! success) UNIVERSAL_LOG("move_new_vars_file_into_place() failed"); } if (success) diff --git a/src/env_universal_common.h b/src/env_universal_common.h index 203f952b6..3428c6937 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -168,7 +168,7 @@ public: }; bool universal_log_enabled(); -#define UNIVERSAL_LOG(x) if (universal_log_enabled()) fprintf(stderr, "UNIVERSAL LOG: %s\n", x) +#define UNIVERSAL_LOG(x) do { if (universal_log_enabled()) fprintf(stderr, "UNIVERSAL LOG: %s\n", x); } while (0) /* Environment variable for requesting a particular universal notifier. See fetch_default_strategy_from_environment for names. */ #define UNIVERSAL_NOTIFIER_ENV_NAME "fish_universal_notifier" diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index cef1857f9..41a0a90fb 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2563,7 +2563,7 @@ static int test_universal_helper(int *x) bool synced = uvars.sync(NULL); if (! synced) { - err(L"Failed to sync universal variables"); + err(L"Failed to sync universal variables after modification"); } fputc('.', stderr); } @@ -2573,7 +2573,7 @@ static int test_universal_helper(int *x) bool synced = uvars.sync(NULL); if (! synced) { - err(L"Failed to sync universal variables"); + err(L"Failed to sync universal variables after deletion"); } fputc('.', stderr); diff --git a/src/wutil.cpp b/src/wutil.cpp index 0e233ac78..a3a51dac7 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -545,15 +545,20 @@ file_id_t file_id_t::file_id_from_stat(const struct stat *buf) result.inode = buf->st_ino; result.size = buf->st_size; result.change_seconds = buf->st_ctime; + result.mod_seconds = buf->st_mtime; #if STAT_HAVE_NSEC result.change_nanoseconds = buf->st_ctime_nsec; + result.mod_nanoseconds = buf->st_mtime_nsec; #elif defined(__APPLE__) result.change_nanoseconds = buf->st_ctimespec.tv_nsec; + result.mod_nanoseconds = buf->st_mtimespec.tv_nsec; #elif defined(_BSD_SOURCE) || defined(_SVID_SOURCE) || defined(_XOPEN_SOURCE) result.change_nanoseconds = buf->st_ctim.tv_nsec; + result.mod_nanoseconds = buf->st_mtim.tv_nsec; #else result.change_nanoseconds = 0; + result.mod_nanoseconds = 0; #endif #if defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) @@ -590,12 +595,7 @@ file_id_t file_id_for_path(const wcstring &path) bool file_id_t::operator==(const file_id_t &rhs) const { - return device == rhs.device && - inode == rhs.inode && - size == rhs.size && - change_seconds == rhs.change_seconds && - change_nanoseconds == rhs.change_nanoseconds && - generation == rhs.generation; + return this->compare_file_id(rhs) == 0; } bool file_id_t::operator!=(const file_id_t &rhs) const @@ -617,7 +617,7 @@ int compare(T a, T b) return 0; } -bool file_id_t::operator<(const file_id_t &rhs) const +int file_id_t::compare_file_id(const file_id_t &rhs) const { /* Compare each field, stopping when we get to a non-equal field */ int ret = 0; @@ -627,5 +627,13 @@ bool file_id_t::operator<(const file_id_t &rhs) const if (! ret) ret = compare(generation, rhs.generation); if (! ret) ret = compare(change_seconds, rhs.change_seconds); if (! ret) ret = compare(change_nanoseconds, rhs.change_nanoseconds); - return ret < 0; + if (! ret) ret = compare(mod_seconds, rhs.mod_seconds); + if (! ret) ret = compare(mod_nanoseconds, rhs.mod_nanoseconds); + return ret; +} + + +bool file_id_t::operator<(const file_id_t &rhs) const +{ + return this->compare_file_id(rhs) < 0; } diff --git a/src/wutil.h b/src/wutil.h index 5c9a0ec2c..9897439a0 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -143,6 +143,8 @@ struct file_id_t uint64_t size; time_t change_seconds; long change_nanoseconds; + time_t mod_seconds; + long mod_nanoseconds; uint32_t generation; bool operator==(const file_id_t &rhs) const; @@ -152,6 +154,9 @@ struct file_id_t bool operator<(const file_id_t &rhs) const; static file_id_t file_id_from_stat(const struct stat *buf); + + private: + int compare_file_id(const file_id_t &rhs) const; }; file_id_t file_id_for_fd(int fd);