Make more miscellaneous globals thread safe

build_tools/find_globals.fish

@@ -74,18 +74,18 @@ end
 function decl_is_threadsafe
     set -l vardecl $argv[1]
     # decls starting with 'const ' or containing ' const ' are assumed safe.
-    string match -q --regex '(^| )const ' $vardecl
+    string match -q --regex '(^|\\*| )const ' $vardecl
     and return 0
 
     # Ordinary types indicating a safe variable.
-    set safes relaxed_atomic_bool_t std::mutex std::condition_variable
+    set safes relaxed_atomic_bool_t std::mutex std::condition_variable std::once_flag sig_atomic_t
     for safe in $safes
         string match -q "*$safe*" $vardecl
         and return 0
     end
 
     # Template types indicate a safe variable.
-    set safes owning_lock mainthread_t std::atomic relaxed_atomic_t
+    set safes owning_lock mainthread_t std::atomic relaxed_atomic_t latch_t
     for safe in $safes
         string match -q "*$safe<*" $vardecl
         and return 0

src/env.cpp

@@ -167,8 +167,8 @@ env_var_t::env_var_flags_t env_var_t::flags_for(const wchar_t *name) {
 
 /// \return a singleton empty list, to avoid unnecessary allocations in env_var_t.
 std::shared_ptr<const wcstring_list_t> env_var_t::empty_list() {
-    static const auto result = std::make_shared<const wcstring_list_t>();
+    static const auto s_empty_result = std::make_shared<const wcstring_list_t>();
-    return result;
+    return s_empty_result;
 }
 
 environment_t::~environment_t() = default;

src/env_dispatch.cpp

@@ -39,6 +39,7 @@
 #include "event.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "function.h"
+#include "global_safety.h"
 #include "history.h"
 #include "input_common.h"
 #include "maybe.h"
@@ -115,7 +116,7 @@ static void init_locale(const environment_t &vars);
 static void update_fish_color_support(const environment_t &vars);
 
 /// True if we think we can set the terminal title.
-static bool can_set_term_title = false;
+static relaxed_atomic_bool_t can_set_term_title{false};
 
 // Run those dispatch functions which want to be run at startup.
 static void run_inits(const environment_t &vars);
@@ -126,7 +127,7 @@ static std::unique_ptr<const var_dispatch_table_t> create_dispatch_table();
 
 // A pointer to the variable dispatch table. This is allocated with new() and deliberately leaked to
 // avoid shutdown destructors. This is set during startup and should not be modified after.
-static const var_dispatch_table_t *s_var_dispatch_table;
+static latch_t<const var_dispatch_table_t> s_var_dispatch_table;
 
 void env_dispatch_init(const environment_t &vars) {
     run_inits(vars);

src/global_safety.h

@@ -63,11 +63,17 @@ class latch_t : detail::fixed_t {
     T *operator->() { return value_; }
     const T *operator->() const { return value_; }
 
-    template <typename... Args>
+    void operator=(T *value) {
-    void emplace(Args &&... args) {
         ASSERT_IS_MAIN_THREAD();
         assert(value_ == nullptr && "Latch variable initialized multiple times");
-        value_ = new T(std::forward<Args>(args)...);
+        assert(value != nullptr && "Latch variable initialized with null");
+        value_ = value;
+    }
+
+    template <typename... Args>
+    void emplace(Args &&... args) {
+        // Note: deliberate leak.
+        *this = new T(std::forward<Args>(args)...);
     }
 };
 
@@ -83,6 +89,12 @@ class relaxed_atomic_t {
     operator T() const { return value_.load(std::memory_order_relaxed); }
 
     void operator=(T v) { return value_.store(v, std::memory_order_relaxed); }
+
+    // postincrement
+    T operator++(int) { return value_.fetch_add(1, std::memory_order_relaxed); }
+
+    // preincrement
+    T operator++() { return 1 + value_.fetch_add(1, std::memory_order_relaxed); }
 };
 
 using relaxed_atomic_bool_t = relaxed_atomic_t<bool>;

src/history.cpp

@@ -31,13 +31,14 @@
 #include "common.h"
 #include "env.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "global_safety.h"
 #include "history.h"
 #include "io.h"
 #include "iothread.h"
 #include "lru.h"
-#include "parser.h"
 #include "parse_constants.h"
 #include "parse_util.h"
+#include "parser.h"
 #include "path.h"
 #include "reader.h"
 #include "tnode.h"
@@ -858,7 +859,7 @@ void history_t::save_internal_unless_disabled() {
     // the counter.
     const int kVacuumFrequency = 25;
     if (countdown_to_vacuum < 0) {
-        static unsigned int seed = (unsigned int)time(NULL);
+        unsigned int seed = (unsigned int)time(NULL);
         // Generate a number in the range [0, kVacuumFrequency).
         countdown_to_vacuum = rand_r(&seed) / (RAND_MAX / kVacuumFrequency + 1);
     }
@@ -1939,8 +1940,8 @@ void history_t::add_pending_with_file_detection(const wcstring &str,
     history_identifier_t identifier = 0;
     if (!potential_paths.empty() && !impending_exit) {
         // Grab the next identifier.
-        static history_identifier_t sLastIdentifier = 0;
+        static relaxed_atomic_t<history_identifier_t> s_last_identifier{0};
-        identifier = ++sLastIdentifier;
+        identifier = ++s_last_identifier;
 
         // Prevent saving until we're done, so we have time to get the paths.
         this->disable_automatic_saving();

src/history.h

@@ -57,7 +57,7 @@ enum history_search_type_t {
     HISTORY_SEARCH_TYPE_PREFIX_GLOB
 };
 
-typedef uint32_t history_identifier_t;
+typedef uint64_t history_identifier_t;
 
 class history_item_t {
     friend class history_t;

src/parse_execution.cpp

@@ -795,7 +795,6 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(
     }
 
     // Protect against exec with background processes running
-    static uint32_t last_exec_run_counter =  -1;
     if (process_type == process_type_t::exec && shell_is_interactive()) {
         bool have_bg = false;
         for (const auto &bg : jobs()) {
@@ -809,13 +808,13 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(
         }
 
         if (have_bg) {
-            /* debug(1, "Background jobs remain! run_counter: %u, last_exec_run_count: %u", reader_run_count(), last_exec_run_counter); */
+            uint64_t current_run_count = reader_run_count();
-            if (isatty(STDIN_FILENO) && reader_run_count() - 1 != last_exec_run_counter) {
+            uint64_t &last_exec_run_count = parser->libdata().last_exec_run_counter;
+            if (isatty(STDIN_FILENO) && current_run_count - 1 != last_exec_run_count) {
                 reader_bg_job_warning();
-                last_exec_run_counter = reader_run_count();
+                last_exec_run_count = current_run_count;
                 return parse_execution_errored;
-            }
+            } else {
-            else {
                 hup_background_jobs();
             }
         }

src/parser.h

@@ -152,6 +152,9 @@ struct library_data_t {
     /// A counter incremented every time a command executes.
     uint64_t exec_count{0};
 
+    /// Last reader run count.
+    uint64_t last_exec_run_counter{UINT64_MAX};
+
     /// Number of recursive calls to builtin_complete().
     uint32_t builtin_complete_recursion_level{0};
 };

src/reader.cpp

@@ -53,6 +53,7 @@
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "function.h"
+#include "global_safety.h"
 #include "highlight.h"
 #include "history.h"
 #include "input.h"
@@ -500,17 +501,15 @@ static struct termios terminal_mode_on_startup;
 static struct termios tty_modes_for_external_cmds;
 
 /// Tracks a currently pending exit. This may be manipulated from a signal handler.
-struct {
-    /// Whether we should exit the current reader loop.
-    bool end_current_loop{false};
 
-    /// Whether we should exit all reader loops. This is set in response to a HUP signal and it
+/// Whether we should exit the current reader loop.
-    /// latches (once set it is never cleared). This should never be reset to false.
+static relaxed_atomic_bool_t s_end_current_loop{false};
-    volatile bool force{false};
 
-    bool should_exit() const { return end_current_loop || force; }
+/// Whether we should exit all reader loops. This is set in response to a HUP signal and it
+/// latches (once set it is never cleared). This should never be reset to false.
+static volatile sig_atomic_t s_exit_forced{false};
 
-} s_pending_exit;
+static bool should_exit() { return s_end_current_loop || s_exit_forced; }
 
 /// Give up control of terminal.
 static void term_donate(outputter_t &outp) {
@@ -546,7 +545,7 @@ static void term_steal() {
     invalidate_termsize();
 }
 
-bool reader_exit_forced() { return s_pending_exit.force; }
+bool reader_exit_forced() { return s_exit_forced; }
 
 /// Given a command line and an autosuggestion, return the string that gets shown to the user.
 wcstring combine_command_and_autosuggestion(const wcstring &cmdline,
@@ -1016,11 +1015,11 @@ void restore_term_mode() {
 }
 
 /// Exit the current reader loop. This may be invoked from a signal handler.
-void reader_set_end_loop(bool flag) { s_pending_exit.end_current_loop = flag; }
+void reader_set_end_loop(bool flag) { s_end_current_loop = flag; }
 
 void reader_force_exit() {
     // Beware, we may be in a signal handler.
-    s_pending_exit.force = true;
+    s_exit_forced = true;
 }
 
 /// Indicates if the given command char ends paging.
@@ -2139,7 +2138,7 @@ void reader_pop() {
     if (new_reader == nullptr) {
         reader_interactive_destroy();
     } else {
-        s_pending_exit.end_current_loop = false;
+        s_end_current_loop = false;
         s_reset(&new_reader->screen, screen_reset_abandon_line);
     }
 }
@@ -2192,7 +2191,7 @@ void reader_import_history_if_necessary() {
     }
 }
 
-bool shell_is_exiting() { return s_pending_exit.should_exit(); }
+bool shell_is_exiting() { return should_exit(); }
 
 void reader_bg_job_warning() {
     std::fputws(_(L"There are still jobs active:\n"), stdout);
@@ -2254,10 +2253,10 @@ static bool selection_is_at_top() {
     return true;
 }
 
-static uint32_t run_count = 0;
+static relaxed_atomic_t<uint64_t> run_count{0};
 
 /// Returns the current interactive loop count
-uint32_t reader_run_count() { return run_count; }
+uint64_t reader_run_count() { return run_count; }
 
 /// Read interactively. Read input from stdin while providing editing facilities.
 static int read_i() {

src/reader.h

@@ -236,6 +236,6 @@ void reader_bg_job_warning();
 
 /// Return the current interactive reads loop count. Useful for determining how many commands have
 /// been executed between invocations of code.
-uint32_t reader_run_count();
+uint64_t reader_run_count();
 
 #endif

src/sanity.cpp

@@ -5,6 +5,7 @@
 
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "global_safety.h"
 #include "history.h"
 #include "kill.h"
 #include "proc.h"
@@ -12,7 +13,7 @@
 #include "sanity.h"
 
 /// Status from earlier sanity checks.
-static bool insane = false;
+static relaxed_atomic_bool_t insane{false};
 
 void sanity_lose() {
     debug(0, _(L"Errors detected, shutting down. Break on sanity_lose() to debug."));

src/wutil.cpp

@@ -7,7 +7,6 @@
 #include <fcntl.h>
 #include <libgen.h>
 #include <limits.h>
-#include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <cstring>
@@ -22,6 +21,7 @@
 #include <cwchar>
 #include <wctype.h>
 
+#include <atomic>
 #include <string>
 #include <unordered_map>
 
@@ -534,8 +534,8 @@ static void wgettext_really_init() {
 /// For wgettext: Internal init function. Automatically called when a translation is first
 /// requested.
 static void wgettext_init_if_necessary() {
-    static pthread_once_t once = PTHREAD_ONCE_INIT;
+    static std::once_flag s_wgettext_init{};
-    pthread_once(&once, wgettext_really_init);
+    std::call_once(s_wgettext_init, wgettext_really_init);
 }
 
 const wcstring &wgettext(const wchar_t *in) {
@@ -614,7 +614,7 @@ int fish_wcswidth(const wchar_t *str) { return fish_wcswidth(str, std::wcslen(st
 int fish_wcswidth(const wcstring &str) { return fish_wcswidth(str.c_str(), str.size()); }
 
 locale_t fish_c_locale() {
-    static locale_t loc = newlocale(LC_ALL_MASK, "C", NULL);
+    static const locale_t loc = newlocale(LC_ALL_MASK, "C", NULL);
     return loc;
 }