diff --git a/fish_tests.cpp b/fish_tests.cpp index 0f7358b23..f0ee316d4 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -1117,7 +1117,7 @@ void history_tests_t::test_history_races(void) delete hist; // Test concurrent history writing -#define RACE_COUNT 20 +#define RACE_COUNT 10 pid_t children[RACE_COUNT]; for (size_t i=0; i < RACE_COUNT; i++) @@ -1186,7 +1186,10 @@ void history_tests_t::test_history_races(void) break; } } - assert(i < RACE_COUNT && "Line expected to be at end of some array"); + if (i >= RACE_COUNT) + { + err(L"Line '%ls' found in history not found in some array", item.str().c_str()); + } } // every write should add at least one item assert(hist_idx >= RACE_COUNT); @@ -1428,7 +1431,6 @@ int main(int argc, char **argv) reader_init(); env_init(); -#if 0 test_format(); test_escape(); test_convert(); @@ -1442,11 +1444,9 @@ int main(int argc, char **argv) test_is_potential_path(); test_colors(); test_autosuggest_suggest_special(); -#endif history_tests_t::test_history(); history_tests_t::test_history_merge(); history_tests_t::test_history_races(); - return 0; history_tests_t::test_history_formats(); say(L"Encountered %d errors in low-level tests", err_count); diff --git a/history.cpp b/history.cpp index c67402be1..6641c0ae9 100644 --- a/history.cpp +++ b/history.cpp @@ -531,7 +531,13 @@ void history_t::add(const history_item_t &item) void history_t::add(const wcstring &str, const path_list_t &valid_paths) { - this->add(history_item_t(str, time(NULL), valid_paths)); + time_t when = time(NULL); + /* Big hack: do not allow timestamps equal to our birthdate. This is because we include items whose timestamps are equal to our birthdate when reading old history, so we can catch "just closed" items. But this means that we may interpret our own items, that we just wrote, as old items, if we wrote them in the same second as our birthdate. + */ + if (when == this->birth_timestamp) + when++; + + this->add(history_item_t(str, when, valid_paths)); } void history_t::remove(const wcstring &str) @@ -1168,8 +1174,8 @@ bool history_t::save_internal_via_rewrite() bool ok = true; - wcstring tmp_name = history_filename(name, L".tmp"); - if (! tmp_name.empty()) + wcstring tmp_name_template = history_filename(name, L".XXXXXX"); + if (! tmp_name_template.empty()) { /* Make an LRU cache to save only the last N elements */ history_lru_cache_t lru(HISTORY_SAVE_MAX); @@ -1227,9 +1233,24 @@ bool history_t::save_internal_via_rewrite() signal_block(); - int out_fd = wopen_cloexec(tmp_name, O_WRONLY | O_CREAT | O_TRUNC, 0644); + /* Try to create a temporary file, up to 10 times. We don't use mkstemps because we want to open it CLO_EXEC. This should almost always succeed on the first try. */ + int out_fd = -1; + wcstring tmp_name; + for (size_t attempt = 0; attempt < 10 && out_fd == -1; attempt++) + { + char *narrow_str = wcs2str(tmp_name_template.c_str()); + if (narrow_str && mktemp(narrow_str)) + { + /* It was successfully templated; try opening it atomically */ + tmp_name = str2wcstring(narrow_str); + out_fd = wopen_cloexec(tmp_name, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, 0644); + } + free(narrow_str); + } + if (out_fd >= 0) { + /* Success */ FILE *out = fdopen(out_fd, "w"); if (out) { diff --git a/history.h b/history.h index 4eec15164..a446c0074 100644 --- a/history.h +++ b/history.h @@ -14,6 +14,15 @@ #include #include +/* fish supports multiple shells writing to history at once. Here is its strategy: + +1. All history files are append-only. Data, once written, is never modified. +2. A history file may be re-written ("vacuumed"). This involves reading in the file and writing a new one, while performing maintenance tasks: discarding items in an LRU fashion until we reach the desired maximum count, removing duplicates, and sorting them by timestamp (eventually, not implemented yet). The new file is atomically moved into place via rename(). +3. History files are mapped in via mmap(). Before the file is mapped, the file takes a fcntl read lock. The purpose of this lock is to avoid seeing a transient state where partial data has been written to the file. +4. History is appended to under a fcntl write lock. +5. The chaos_mode boolean can be set to true to do things like lower buffer sizes which can trigger race conditions. This is useful for testing. +*/ + typedef std::vector path_list_t; enum history_search_type_t