Remove fish_mutex_t wrapper around std::mutex

@ridiculousfish had introduced this in 3a45cad12e
to work around an issue with Coverity Scan where it couldn't tell the
mutex was correctly locked, but even with the `fish_mutex_t` hack, it
still emits the same warnings, so there's no pointing in keeping it.

src/autoload.h

@@ -46,7 +46,7 @@ class env_vars_snapshot_t;
 class autoload_t : public lru_cache_t<autoload_t, autoload_function_t> {
    private:
     /// Lock for thread safety.
-    fish_mutex_t lock;
+    std::mutex lock;
     /// The environment variable name.
     const wcstring env_var_name;
     /// The paths from which to autoload, or missing if none.

src/common.cpp

@@ -78,7 +78,7 @@ static pid_t initial_fg_process_group = -1;
 /// This struct maintains the current state of the terminal size. It is updated on demand after
 /// receiving a SIGWINCH. Do not touch this struct directly, it's managed with a rwlock. Use
 /// common_get_width()/common_get_height().
-static fish_mutex_t termsize_lock;
+static std::mutex termsize_lock;
 static struct winsize termsize = {USHRT_MAX, USHRT_MAX, USHRT_MAX, USHRT_MAX};
 static volatile bool termsize_valid = false;
 
@@ -2234,11 +2234,16 @@ void assert_is_background_thread(const char *who) {
     }
 }
 
-void fish_mutex_t::assert_is_locked(const char *who, const char *caller) const {
-    if (!is_locked_) {
+void assert_is_locked(void *vmutex, const char *who, const char *caller) {
+    std::mutex *mutex = static_cast<std::mutex *>(vmutex);
+
+    // Note that std::mutex.try_lock() is allowed to return false when the mutex isn't
+    // actually locked; fortunately we are checking the opposite so we're safe.
+    if (mutex->try_lock()) {
         debug(0, "%s is not locked when it should be in '%s'", who, caller);
         debug(0, "Break on debug_thread_error to debug.");
         debug_thread_error();
+        mutex->unlock();
     }
 }
 

src/common.h

@@ -543,39 +543,10 @@ void assert_is_background_thread(const char *who);
 #define ASSERT_IS_BACKGROUND_THREAD_TRAMPOLINE(x) assert_is_background_thread(x)
 #define ASSERT_IS_BACKGROUND_THREAD() ASSERT_IS_BACKGROUND_THREAD_TRAMPOLINE(__FUNCTION__)
 
-// fish_mutex is a wrapper around std::mutex that tracks whether it is locked, allowing for checking
-// if the mutex is locked. It owns a boolean guarded by the lock that records whether the lock is
-// currently locked; this is only used by assertions for correctness.
-class fish_mutex_t {
-    std::mutex lock_{};
-    bool is_locked_{false};
-
-   public:
-    constexpr fish_mutex_t() = default;
-    ~fish_mutex_t() = default;
-
-    void lock() {
-        lock_.lock();
-        is_locked_ = true;
-    }
-
-    void unlock() {
-        is_locked_ = false;
-        lock_.unlock();
-    }
-
-    // assert that this lock (identified as 'who') is locked in the function 'caller'.
-    void assert_is_locked(const char *who, const char *caller) const;
-
-    // return the underlying std::mutex. Note the fish_mutex_t cannot track locks to the underlying
-    // mutex; do not use assert_is_locked() with this.
-    std::mutex &get_mutex() { return lock_; }
-};
-
 /// Useful macro for asserting that a lock is locked. This doesn't check whether this thread locked
 /// it, which it would be nice if it did, but here it is anyways.
-void assert_is_locked(const fish_mutex_t &m, const char *who, const char *caller);
-#define ASSERT_IS_LOCKED(x) (x).assert_is_locked(#x, __FUNCTION__)
+void assert_is_locked(void *mutex, const char *who, const char *caller);
+#define ASSERT_IS_LOCKED(x) assert_is_locked((void *)(&x), #x, __FUNCTION__)
 
 /// Format the specified size (in bytes, kilobytes, etc.) into the specified stringbuffer.
 wcstring format_size(long long sz);
@@ -706,7 +677,7 @@ class null_terminated_array_t {
 // null_terminated_array_t<char_t>.
 void convert_wide_array_to_narrow(const null_terminated_array_t<wchar_t> &arr,
                                   null_terminated_array_t<char> *output);
-typedef std::lock_guard<fish_mutex_t> scoped_lock;
+typedef std::lock_guard<std::mutex> scoped_lock;
 typedef std::lock_guard<std::recursive_mutex> scoped_rlock;
 
 // An object wrapping a scoped lock and a value
@@ -721,8 +692,8 @@ typedef std::lock_guard<std::recursive_mutex> scoped_rlock;
 //
 template <typename DATA>
 class acquired_lock {
-    std::unique_lock<fish_mutex_t> lock;
-    acquired_lock(fish_mutex_t &lk, DATA *v) : lock(lk), value(v) {}
+    std::unique_lock<std::mutex> lock;
+    acquired_lock(std::mutex &lk, DATA *v) : lock(lk), value(v) {}
 
     template <typename T>
     friend class owning_lock;
@@ -752,7 +723,7 @@ class owning_lock {
     owning_lock(owning_lock &&) = default;
     owning_lock &operator=(owning_lock &&) = default;
 
-    fish_mutex_t lock;
+    std::mutex lock;
     DATA data;
 
    public:

src/complete.cpp

@@ -1253,9 +1253,7 @@ bool completer_t::try_complete_user(const wcstring &str) {
     bool result = false;
     size_t name_len = str.length() - 1;
 
-    // We don't bother with the thread-safe `getpwent_r()` variant because this is the sole place
-    // where we call getpwent().
-    static fish_mutex_t lock;
+    static std::mutex lock;
     scoped_lock locker(lock);
     setpwent();
     // cppcheck-suppress getpwentCalled

src/env.cpp

@@ -139,7 +139,7 @@ class env_node_t {
     bool contains_any_of(const wcstring_list_t &vars) const;
 };
 
-static fish_mutex_t env_lock;
+static std::mutex env_lock;
 
 static bool local_scope_exports(const env_node_t *n);
 

src/env_universal_common.h

@@ -52,7 +52,7 @@ class env_universal_t {
     // fish wrote the uvars contents.
     bool ok_to_save{true};
 
-    mutable fish_mutex_t lock;
+    mutable std::mutex lock;
     bool load_from_path(const wcstring &path, callback_data_list_t &callbacks);
     void load_from_fd(int fd, callback_data_list_t &callbacks);
 

src/history.h

@@ -122,7 +122,7 @@ class history_t {
     void add(const history_item_t &item, bool pending = false);
 
     // Lock for thread safety.
-    fish_mutex_t lock;
+    std::mutex lock;
 
     // Internal function.
     void clear_file_state();

src/iothread.cpp

@@ -72,9 +72,9 @@ static owning_lock<thread_data_t> s_spawn_requests;
 static owning_lock<std::queue<spawn_request_t>> s_result_queue;
 
 // "Do on main thread" support.
-static fish_mutex_t s_main_thread_performer_lock;             // protects the main thread requests
+static std::mutex s_main_thread_performer_lock;               // protects the main thread requests
 static std::condition_variable s_main_thread_performer_cond;  // protects the main thread requests
-static fish_mutex_t s_main_thread_request_q_lock;             // protects the queue
+static std::mutex s_main_thread_request_q_lock;               // protects the queue
 static std::queue<main_thread_request_t *> s_main_thread_request_queue;
 
 // Notifying pipes.
@@ -332,7 +332,7 @@ void iothread_perform_on_main(void_function_t &&func) {
     assert_with_errno(write_loop(s_write_pipe, &wakeup_byte, sizeof wakeup_byte) != -1);
 
     // Wait on the condition, until we're done.
-    std::unique_lock<std::mutex> perform_lock(s_main_thread_performer_lock.get_mutex());
+    std::unique_lock<std::mutex> perform_lock(s_main_thread_performer_lock);
     while (!req.done) {
         // It would be nice to support checking for cancellation here, but the clients need a
         // deterministic way to clean up to avoid leaks