Support using condition variables

.github/workflows/cmake.yml

@@ -142,18 +142,6 @@ jobs:
               os: windows-latest,
               compiler: { type: VISUAL, version: 19, cc: "cl", cxx: "cl" },
             }
-          - {
-              name: "MacOS Apple Clang 15",
-              os: macos-14,
-              compiler:
-                {
-                  type: APPLE_CLANG,
-                  version: "15.0",
-                  cc: "clang",
-                  cxx: "clang++",
-                  std: 20,
-                },
-            }
     steps:
       - uses: actions/checkout@v4
       - uses: seanmiddleditch/gha-setup-ninja@master

DRAFT.md

@@ -77,6 +77,27 @@ the standard library header `<mutex>`.
 
 ## Choices
 
+### Using `lock_guard` or `unique_lock`
+
+Calling `lock` unconditionally locks the mutex, so intuitively it should return a
+`lock_guard`, which is slightly faster than `unique_lock`, which needs to check if
+the lock is held at destruction. The problem with this is that `lock_guard` is
+incompatible with `condition_variable`, which only accept a `unique_lock`.
+Alternatively we could expose the mutex and force people to use
+`condition_variable_any`, but `lock_guard` doesn't expose `mutex` either. We could
+package an extra reference to the underlying mutex, but that would cost additional
+memory (unless the optimizer removes the unused reference).
+
+It seems returning a `unique_lock` is low enough cost to not be a big deal and
+worth the additional compabitility. If the cost is too high, then you can avoid it
+by using `with` instead of `lock`. Since the guard isn't exposed and therefore
+can't be used with a condition variable `with` uses `lock_guard` for the additional
+speed.
+
+One other minor argument for always using a `unique_lock` is so that the return
+type doesn't change if you switch from `lock` to `try_lock`, where `unique_lock`
+is needed.
+
 ### Explicit or overloaded method names
 
 Is it better to be explicit with all the variants, or use overloads? We've
@@ -93,7 +114,7 @@ though the templates, namespaces, `[[nodiscard]]` and concept constraints have
 been ommited for clarity.
 
 ```c++
-  mutex_locked<T, lock_guard<M>> lock();
+  mutex_locked<T, unique_lock<M>> lock();
   mutex_locked<T, unique_lock<M>> try_lock();
   mutex_locked<T, unique_lock<M>> try_lock_until(const time_point &timeout_time);
   mutex_locked<T, unique_lock<M>> try_lock_for(const duration &timeout_duration);
@@ -122,7 +143,7 @@ The alternative is to use overloads. A potential api is below, with the same
 simplifications as above.
 
 ```c++
-  mutex_locked<T, lock_guard<M>> lock();
+  mutex_locked<T, unique_lock<M>> lock();
   mutex_locked<T, unique_lock<M>> lock(const time_point &timeout_time);
   mutex_locked<T, unique_lock<M>> lock(const duration &timeout_duration);
 
@@ -148,7 +169,7 @@ support `try_lock` on a `std::mutex`. This suggests an alternative in
 between may be more reasonable:
 
 ```c++
-  mutex_locked<T, lock_guard<M>> lock();
+  mutex_locked<T, unique_lock<M>> lock();
   mutex_locked<T, unique_lock<M>> try_lock();
   mutex_locked<T, unique_lock<M>> try_lock(const time_point &timeout_time);
   mutex_locked<T, unique_lock<M>> try_lock(const duration &timeout_duration);

README.md

@@ -225,7 +225,42 @@ mutex_protected<int, std::shared_mutex> value;
 }
 ```
 
-### Condition variables
+### Condition Variables
+
+Condition variables are used to wait for a particular change to occur on
+internal data, but need access to either the underlying lock guard or the
+underlying mutex.
+
+#### std::mutex
+
+`std::condition_variable` only works with a `std::mutex`, so when using
+The `mutex_protected<T, std::mutex>` the `mutex_locked` instance has a `guard`
+method that returns the underlying `std::unique_lock`.
+
+```cpp
+mutex_protected<std::vector<int>> data;
+std::condition_variable cv;
+
+// Wait for some other thread to add data to the vector.
+auto locked = data.lock();
+cv.wait(locked.guard(), [&locked]() { return !locked->empty(); });
+```
+
+#### Other mutexes, eg: std::shared_mutex
+
+For all other mutex types, you'll need to use a `std::condition_variable_any`
+which works on the mutex instead of on the guard. For that, you can use the
+`mutex` method on the `mutex_locked`. This works regardless of whether the
+lock is exclusive or shared.
+
+```cpp
+mutex_protected<std::vector<int>, std::shared_mutex> data;
+std::condition_variable_any cv;
+
+// Wait for some other thread to add data to the vector.
+auto locked = data.lock();
+cv.wait(locked.mutex(), [&locked]() { return !locked->empty(); });
+```
 
 ### Locking multiple mutexes simultaneously
 

mutex_protected.h

@@ -52,13 +52,20 @@ class [[nodiscard]] mutex_locked {
   bool owns_lock() const noexcept
     requires std::is_member_function_pointer_v<decltype(&G::owns_lock)>
   {
-    return guard.owns_lock();
+    return g.owns_lock();
   }
 
   operator bool() const noexcept
     requires std::is_member_function_pointer_v<decltype(&G::owns_lock)>
   {
-    return guard.owns_lock();
+    return g.owns_lock();
+  }
+
+  // Needed for use with `std::condition_variable`.
+  G &guard() noexcept
+    requires std::is_same_v<G, std::unique_lock<std::mutex>>
+  {
+    return g;
   }
 
   mutex_locked(const mutex_locked &) = delete;
@@ -67,15 +74,22 @@ class [[nodiscard]] mutex_locked {
 
   mutex_locked(mutex_locked &&m) noexcept
     requires std::move_constructible<G>
-      : v(std::exchange(m.v, nullptr)), guard(std::move(m.guard)) {}
+      : v(std::exchange(m.v, nullptr)), g(std::move(m.g)) {}
+
+  // Needed for use with `std::condition_variable_any`, ie if you are using a
+  // different mutex type.
+  mutex_type *mutex() noexcept
+    requires std::is_member_function_pointer_v<decltype(&G::mutex)>
+  {
+    return g.mutex();
+  }
 
  private:
   template <typename... Args>
-  mutex_locked(T *v_, Args &&...args)
-      : v{v_}, guard{std::forward<Args>(args)...} {}
+  mutex_locked(T *v_, Args &&...args) : v{v_}, g{std::forward<Args>(args)...} {}
 
   T *v;
-  G guard;
+  G g;
 
   template <class T_, Mutex M_>
   friend class mutex_protected;
@@ -92,8 +106,10 @@ class mutex_protected {
 
   mutex_protected(const T &v_) : mutex{}, v(v_) {}
 
-  mutex_locked<T, std::lock_guard<M>> lock() {
-    return mutex_locked<T, std::lock_guard<M>>(&v, mutex);
+  // Returning a `lock_guard` would be slightly faster, but wouldn't work with a
+  // condition variable.
+  mutex_locked<T, std::unique_lock<M>> lock() {
+    return mutex_locked<T, std::unique_lock<M>>(&v, mutex);
   }
 
   mutex_locked<T, std::unique_lock<M>> try_lock() {

mutex_protected_test.cc

@@ -1,6 +1,8 @@
 #include "mutex_protected.h"
 
 #include <chrono>
+#include <condition_variable>
+#include <queue>
 #include <string>
 #include <thread>
 #include <type_traits>
@@ -461,4 +463,89 @@ TEST(SharedTimedMutexProtectedTest, TimeoutWorksCorrectly) {
   EXPECT_EQ(*write_locked, 1);
 }
 
+TEST(CondVarMutexProtectedTest, ConditionVariableWorks) {
+  mutex_protected<std::queue<int>> inputs;
+  mutex_protected<std::queue<int>> outputs;
+  std::condition_variable in_cv;
+  std::condition_variable out_cv;
+  std::stop_source stop;
+
+  const int items = 100;
+  const int thread_count = 2;
+
+  std::vector<std::thread> threads;
+  threads.reserve(thread_count);
+  for (int i = 0; i < thread_count; ++i) {
+    threads.emplace_back([&]() {
+      while (!stop.stop_requested()) {
+        int value;
+        {
+          auto locked = inputs.lock();
+          in_cv.wait(locked.guard(), [&stop, &locked]() {
+            return stop.stop_requested() || !locked->empty();
+          });
+          if (locked->empty()) {  // Stop requested, exit
+            return;
+          }
+          value = locked->front();
+          locked->pop();
+        }
+        outputs.lock()->push(value);
+        out_cv.notify_one();
+      }
+    });
+  }
+
+  std::this_thread::sleep_for(5ms);
+  int expected_total = 0;
+  for (int i = 0; i < items; ++i) {
+    inputs.with([&i](auto& in_q) { in_q.push(i); });
+    in_cv.notify_one();
+    expected_total += i;
+  }
+
+  int total = 0;
+  for (int i = 0; i < items; ++i) {
+    auto locked = outputs.lock();
+    out_cv.wait(locked.guard(), [&locked]() { return !locked->empty(); });
+    total += locked->front();
+    locked->pop();
+  }
+
+  stop.request_stop();
+  in_cv.notify_all();
+  for (auto& thread : threads) {
+    thread.join();
+  }
+
+  EXPECT_EQ(total, expected_total);
+}
+
+TEST(CondVarMutexProtectedTest, ConditionVariableAnyWorks) {
+  mutex_protected<int, std::shared_mutex> data = 0;
+  std::condition_variable_any cv;
+  mutex_protected<int> out = 0;
+
+  const int thread_count = 2;
+
+  std::vector<std::thread> threads;
+  threads.reserve(thread_count);
+  for (int i = 0; i < thread_count; ++i) {
+    threads.emplace_back([&]() {
+      auto locked = data.lock_shared();
+      cv.wait(*locked.mutex(), [&locked]() { return *locked > 0; });
+      EXPECT_EQ(*locked, 1);
+      *out.lock() += *locked;
+    });
+  }
+
+  std::this_thread::sleep_for(5ms);
+  *data.lock() = 1;
+  cv.notify_all();
+  for (auto& thread : threads) {
+    thread.join();
+  }
+  EXPECT_EQ(*out.lock(), thread_count);
+}
+
 }  // namespace xyz