[libcxx-commits] [libcxx] [libc++] Refactor tests for std::condition_variable (PR #91530)

[via libcxx-commits]

================
@@ -20,82 +19,142 @@
 //              Predicate pred);
 
 #include <condition_variable>
+#include <atomic>
+#include <cassert>
+#include <chrono>
 #include <mutex>
 #include <thread>
-#include <chrono>
-#include <cassert>
 
 #include "make_test_thread.h"
 #include "test_macros.h"
 
-class Pred
-{
-    int& i_;
-public:
-    explicit Pred(int& i) : i_(i) {}
-
-    bool operator()() {return i_ != 0;}
-};
-
-std::condition_variable cv;
-std::mutex mut;
-
-int test1 = 0;
-int test2 = 0;
-
-int runs = 0;
-
-void f()
-{
-    typedef std::chrono::system_clock Clock;
-    typedef std::chrono::milliseconds milliseconds;
-    std::unique_lock<std::mutex> lk(mut);
-    assert(test2 == 0);
-    test1 = 1;
-    cv.notify_one();
-    Clock::time_point t0 = Clock::now();
-    bool r = cv.wait_for(lk, milliseconds(250), Pred(test2));
-    ((void)r); // Prevent unused warning
-    Clock::time_point t1 = Clock::now();
-    if (runs == 0)
-    {
-        assert(t1 - t0 < milliseconds(250));
-        assert(test2 != 0);
-    }
-    else
-    {
-        assert(t1 - t0 - milliseconds(250) < milliseconds(50));
-        assert(test2 == 0);
-    }
-    ++runs;
+template <class Function>
+std::chrono::microseconds measure(Function f) {
+  std::chrono::high_resolution_clock::time_point start = std::chrono::high_resolution_clock::now();
+  f();
+  std::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();
+  return std::chrono::duration_cast<std::chrono::microseconds>(end - start);
 }
 
-int main(int, char**)
-{
-    {
-        std::unique_lock<std::mutex>lk(mut);
-        std::thread t = support::make_test_thread(f);
-        assert(test1 == 0);
-        while (test1 == 0)
-            cv.wait(lk);
-        assert(test1 != 0);
-        test2 = 1;
-        lk.unlock();
-        cv.notify_one();
-        t.join();
-    }
-    test1 = 0;
-    test2 = 0;
-    {
-        std::unique_lock<std::mutex>lk(mut);
-        std::thread t = support::make_test_thread(f);
-        assert(test1 == 0);
-        while (test1 == 0)
-            cv.wait(lk);
-        assert(test1 != 0);
-        lk.unlock();
-        t.join();
-    }
+int main(int, char**) {
+  // Test unblocking via a call to notify_one() in another thread.
+  //
+  // To test this, we set a very long timeout in wait_for() and we try to minimize
+  // the likelihood that we got awoken by a spurious wakeup by updating the
+  // likely_spurious flag only immediately before we perform the notification.
+  {
+    std::atomic<bool> ready           = false;
+    std::atomic<bool> likely_spurious = true;
+    auto timeout                      = std::chrono::seconds(3600);
+    std::condition_variable cv;
+    std::mutex mutex;
+
+    std::thread t1 = support::make_test_thread([&] {
+      std::unique_lock<std::mutex> lock(mutex);
+      auto elapsed = measure([&] {
+        ready       = true;
+        bool result = cv.wait_for(lock, timeout, [&] { return !likely_spurious; });
+        assert(result); // return value should be true since we didn't time out
+      });
+      assert(elapsed < timeout);
+    });
+
+    std::thread t2 = support::make_test_thread([&] {
+      while (!ready) {
+        // spin
+      }
+
+      // Acquire the same mutex as t1. This ensures that the condition variable has started
+      // waiting (and hence released that mutex). We don't actually need to hold the lock, we
+      // simply use it as a signal that the condition variable has started waiting.
+      std::unique_lock<std::mutex> lock(mutex);
+      lock.unlock();
+
+      likely_spurious = false;
----------------
huixie90 wrote:

Usually the variable is changed while holding the lock to prevent other thread seeing the old value after woken up. I think it is a good idea to do it throughout the PR to prevent some sporadic failures

https://github.com/llvm/llvm-project/pull/91530