Fix for a long standing race where multiple shells can overwrite each others' .tmp files, and lose history.

Added a long description of the incremental history strategy
Fixes https://github.com/fish-shell/fish-shell/issues/371

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);

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)
             {

history.h

@@ -14,6 +14,15 @@
 #include <list>
 #include <set>
 
+/* 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<wcstring> path_list_t;
 
 enum history_search_type_t