[libcxx-commits] [libcxx] [libc++] fix condition_variable_any hangs on stop_request (PR #77127)

via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jan 15 05:45:27 PST 2024


https://github.com/huixie90 updated https://github.com/llvm/llvm-project/pull/77127

>From a497c77e9afd5883fda69d1e48bd4dd329e53d11 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Fri, 5 Jan 2024 18:59:09 +0000
Subject: [PATCH 1/4] [libc++] fix condition_variable_any hangs on stop_request

---
 libcxx/include/condition_variable             |  3 +++
 .../wait_for_token_pred.pass.cpp              | 22 +++++++++++++++++++
 .../wait_token_pred.pass.cpp                  | 22 +++++++++++++++++++
 .../wait_until_token_pred.pass.cpp            | 22 +++++++++++++++++++
 4 files changed, 69 insertions(+)

diff --git a/libcxx/include/condition_variable b/libcxx/include/condition_variable
index cf7a570b6cb635..c512901cfb19ca 100644
--- a/libcxx/include/condition_variable
+++ b/libcxx/include/condition_variable
@@ -131,6 +131,7 @@ public:
 #include <__mutex/mutex.h>
 #include <__mutex/tag_types.h>
 #include <__mutex/unique_lock.h>
+#include <__stop_token/stop_callback.h>
 #include <__stop_token/stop_token.h>
 #include <__utility/move.h>
 #include <version>
@@ -257,6 +258,7 @@ condition_variable_any::wait_for(_Lock& __lock, const chrono::duration<_Rep, _Pe
 
 template <class _Lock, class _Predicate>
 bool condition_variable_any::wait(_Lock& __lock, stop_token __stoken, _Predicate __pred) {
+  stop_callback __cb(__stoken, [this] { notify_all(); });
   while (!__stoken.stop_requested()) {
     if (__pred())
       return true;
@@ -268,6 +270,7 @@ bool condition_variable_any::wait(_Lock& __lock, stop_token __stoken, _Predicate
 template <class _Lock, class _Clock, class _Duration, class _Predicate>
 bool condition_variable_any::wait_until(
     _Lock& __lock, stop_token __stoken, const chrono::time_point<_Clock, _Duration>& __abs_time, _Predicate __pred) {
+  stop_callback __cb(__stoken, [this] { notify_all(); });
   while (!__stoken.stop_requested()) {
     if (__pred())
       return true;
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
index fb3f0287726eea..4bb089f3d2858f 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
@@ -155,6 +155,28 @@ void test() {
     assert(lock.owns_lock());
   }
 
+  // #76807 Hangs in std::condition_variable_any when used with std::stop_token
+  {
+    class MyThread {
+    public:
+      MyThread() {
+        thread_ = support::make_test_jthread([this](std::stop_token st) {
+          while (!st.stop_requested()) {
+            std::unique_lock lock{m_};
+            cv_.wait_for(lock, st, 1h, [] { return false; });
+          }
+        });
+      }
+
+    private:
+      std::mutex m_;
+      std::condition_variable_any cv_;
+      std::jthread thread_;
+    };
+
+    [[maybe_unused]] MyThread my_thread;
+  }
+
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
   // Throws: Any exception thrown by pred.
   {
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp
index 451df9ab7ee287..15a64a141c6d0d 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp
@@ -107,6 +107,28 @@ void test() {
     assert(lock.owns_lock());
   }
 
+  // #76807 Hangs in std::condition_variable_any when used with std::stop_token
+  {
+    class MyThread {
+    public:
+      MyThread() {
+        thread_ = support::make_test_jthread([this](std::stop_token st) {
+          while (!st.stop_requested()) {
+            std::unique_lock lock{m_};
+            cv_.wait(lock, st, [] { return false; });
+          }
+        });
+      }
+
+    private:
+      std::mutex m_;
+      std::condition_variable_any cv_;
+      std::jthread thread_;
+    };
+
+    [[maybe_unused]] MyThread my_thread;
+  }
+
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
   // Throws: Any exception thrown by pred.
   {
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
index 6cdcbe36d98598..3d6f0d3eae25a6 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
@@ -157,6 +157,28 @@ void test() {
     assert(lock.owns_lock());
   }
 
+  // #76807 Hangs in std::condition_variable_any when used with std::stop_token
+  {
+    class MyThread {
+    public:
+      MyThread() {
+        thread_ = support::make_test_jthread([this](std::stop_token st) {
+          while (!st.stop_requested()) {
+            std::unique_lock lock{m_};
+            cv_.wait_until(lock, st, std::chrono::steady_clock::now() + std::chrono::hours(1), [] { return false; });
+          }
+        });
+      }
+
+    private:
+      std::mutex m_;
+      std::condition_variable_any cv_;
+      std::jthread thread_;
+    };
+
+    [[maybe_unused]] MyThread my_thread;
+  }
+
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
   // Throws: Any exception thrown by pred.
   {

>From 52ffe6ecc5ebc52771c76ffdbc8148646e867197 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Tue, 9 Jan 2024 18:02:26 +0000
Subject: [PATCH 2/4] fix stop_requested lock issue

---
 libcxx/include/condition_variable             | 76 ++++++++++++++-----
 .../wait_for_token_pred.pass.cpp              | 26 +++++++
 .../wait_token_pred.pass.cpp                  | 26 +++++++
 .../wait_until_token_pred.pass.cpp            | 26 +++++++
 4 files changed, 136 insertions(+), 18 deletions(-)

diff --git a/libcxx/include/condition_variable b/libcxx/include/condition_variable
index c512901cfb19ca..e6399155d87edd 100644
--- a/libcxx/include/condition_variable
+++ b/libcxx/include/condition_variable
@@ -126,7 +126,6 @@ public:
 #include <__condition_variable/condition_variable.h>
 #include <__config>
 #include <__memory/shared_ptr.h>
-#include <__memory/unique_ptr.h>
 #include <__mutex/lock_guard.h>
 #include <__mutex/mutex.h>
 #include <__mutex/tag_types.h>
@@ -201,19 +200,26 @@ inline void condition_variable_any::notify_all() _NOEXCEPT {
   __cv_.notify_all();
 }
 
-struct __lock_external {
-  template <class _Lock>
-  _LIBCPP_HIDE_FROM_ABI void operator()(_Lock* __m) {
-    __m->lock();
+template <class _Lock>
+struct __unlock_guard {
+  _Lock& __lock_;
+
+  _LIBCPP_HIDE_FROM_ABI __unlock_guard(_Lock& __lock) : __lock_(__lock) { __lock_.unlock(); }
+
+  _LIBCPP_HIDE_FROM_ABI ~__unlock_guard() noexcept // turns exception to std::terminate
+  {
+    __lock_.lock();
   }
+
+  __unlock_guard(const __unlock_guard&)            = delete;
+  __unlock_guard& operator=(const __unlock_guard&) = delete;
 };
 
 template <class _Lock>
 void condition_variable_any::wait(_Lock& __lock) {
   shared_ptr<mutex> __mut = __mut_;
   unique_lock<mutex> __lk(*__mut);
-  __lock.unlock();
-  unique_ptr<_Lock, __lock_external> __lxx(&__lock);
+  __unlock_guard<_Lock> __unlock(__lock);
   lock_guard<unique_lock<mutex> > __lx(__lk, adopt_lock_t());
   __cv_.wait(__lk);
 } // __mut_.unlock(), __lock.lock()
@@ -228,8 +234,7 @@ template <class _Lock, class _Clock, class _Duration>
 cv_status condition_variable_any::wait_until(_Lock& __lock, const chrono::time_point<_Clock, _Duration>& __t) {
   shared_ptr<mutex> __mut = __mut_;
   unique_lock<mutex> __lk(*__mut);
-  __lock.unlock();
-  unique_ptr<_Lock, __lock_external> __lxx(&__lock);
+  __unlock_guard<_Lock> __unlock(__lock);
   lock_guard<unique_lock<mutex> > __lx(__lk, adopt_lock_t());
   return __cv_.wait_until(__lk, __t);
 } // __mut_.unlock(), __lock.lock()
@@ -257,25 +262,60 @@ condition_variable_any::wait_for(_Lock& __lock, const chrono::duration<_Rep, _Pe
 #  if _LIBCPP_STD_VER >= 20 && !defined(_LIBCPP_HAS_NO_EXPERIMENTAL_STOP_TOKEN)
 
 template <class _Lock, class _Predicate>
-bool condition_variable_any::wait(_Lock& __lock, stop_token __stoken, _Predicate __pred) {
-  stop_callback __cb(__stoken, [this] { notify_all(); });
-  while (!__stoken.stop_requested()) {
+bool condition_variable_any::wait(_Lock& __user_lock, stop_token __stoken, _Predicate __pred) {
+  // It is not safe to call the destructor on the other thread while waiting,
+  // even if we copy the shared_ptr<mutex> inside the function,
+  // because the stop_callback needs the internal cv.
+  // In order to make it work, we need to change the ABI to make both
+  // mutex and internal cv to be hold inside the shared_ptr, and
+  // make a local copy inside this function
+
+  if (__stoken.stop_requested())
+    return __pred();
+  std::stop_callback __cb(__stoken, [this] { notify_all(); });
+
+  while (true) {
     if (__pred())
       return true;
-    wait(__lock);
-  }
+
+    // We need to take the internal lock before checking stop_requested,
+    // so that the notification cannot come in between the stop_requested
+    // check and entering the wait.
+    // Note that the stop_callback takes the same internal lock before notifying
+    unique_lock<mutex> __internal_lock(*__mut_);
+    if (__stoken.stop_requested())
+      break;
+
+    __unlock_guard<_Lock> __unlock(__user_lock);
+    unique_lock<mutex> __internal_lock2(std::move(__internal_lock));
+    __cv_.wait(__internal_lock2);
+  } // __mut_.unlock(), __lock.lock()
   return __pred();
 }
 
 template <class _Lock, class _Clock, class _Duration, class _Predicate>
 bool condition_variable_any::wait_until(
-    _Lock& __lock, stop_token __stoken, const chrono::time_point<_Clock, _Duration>& __abs_time, _Predicate __pred) {
+    _Lock& __user_lock,
+    stop_token __stoken,
+    const chrono::time_point<_Clock, _Duration>& __abs_time,
+    _Predicate __pred) {
+  if (__stoken.stop_requested())
+    return __pred();
   stop_callback __cb(__stoken, [this] { notify_all(); });
-  while (!__stoken.stop_requested()) {
+
+  while (true) {
     if (__pred())
       return true;
-    if (wait_until(__lock, __abs_time) == cv_status::timeout)
-      return __pred();
+
+    unique_lock<mutex> __internal_lock(*__mut_);
+    if (__stoken.stop_requested())
+      break;
+
+    __unlock_guard<_Lock> __unlock(__user_lock);
+    unique_lock<mutex> __internal_lock2(std::move(__internal_lock));
+
+    if (__cv_.wait_until(__internal_lock2, __abs_time) == cv_status::timeout)
+      break;
   }
   return __pred();
 }
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
index 4bb089f3d2858f..d06576d7c3dd09 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
@@ -177,6 +177,32 @@ void test() {
     [[maybe_unused]] MyThread my_thread;
   }
 
+  // request_stop potentially in-between check and wait
+  {
+    std::stop_source ss;
+    std::condition_variable_any cv;
+    Mutex mutex;
+    Lock lock{mutex};
+
+    std::atomic_bool pred_started        = false;
+    std::atomic_bool request_stop_called = false;
+    auto thread                          = support::make_test_thread([&]() {
+      pred_started.wait(false);
+      ss.request_stop();
+      request_stop_called.store(true);
+    });
+
+    std::same_as<bool> auto r = cv.wait_for(lock, ss.get_token(), 1h, [&]() {
+      pred_started.store(true);
+      request_stop_called.wait(false);
+      return false;
+    });
+    assert(!r);
+    thread.join();
+
+    assert(lock.owns_lock());
+  }
+
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
   // Throws: Any exception thrown by pred.
   {
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp
index 15a64a141c6d0d..7dceb4e8eb73cb 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp
@@ -129,6 +129,32 @@ void test() {
     [[maybe_unused]] MyThread my_thread;
   }
 
+  // request_stop potentially in-between check and wait
+  {
+    std::stop_source ss;
+    std::condition_variable_any cv;
+    Mutex mutex;
+    Lock lock{mutex};
+
+    std::atomic_bool pred_started        = false;
+    std::atomic_bool request_stop_called = false;
+    auto thread                          = support::make_test_thread([&]() {
+      pred_started.wait(false);
+      ss.request_stop();
+      request_stop_called.store(true);
+    });
+
+    std::same_as<bool> auto r = cv.wait(lock, ss.get_token(), [&]() {
+      pred_started.store(true);
+      request_stop_called.wait(false);
+      return false;
+    });
+    assert(!r);
+    thread.join();
+
+    assert(lock.owns_lock());
+  }
+
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
   // Throws: Any exception thrown by pred.
   {
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
index 3d6f0d3eae25a6..95371aa395f4e6 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
@@ -179,6 +179,32 @@ void test() {
     [[maybe_unused]] MyThread my_thread;
   }
 
+  // request_stop potentially in-between check and wait
+  {
+    std::stop_source ss;
+    std::condition_variable_any cv;
+    Mutex mutex;
+    Lock lock{mutex};
+
+    std::atomic_bool pred_started        = false;
+    std::atomic_bool request_stop_called = false;
+    auto thread                          = support::make_test_thread([&]() {
+      pred_started.wait(false);
+      ss.request_stop();
+      request_stop_called.store(true);
+    });
+
+    std::same_as<bool> auto r = cv.wait_until(lock, ss.get_token(), future, [&]() {
+      pred_started.store(true);
+      request_stop_called.wait(false);
+      return false;
+    });
+    assert(!r);
+    thread.join();
+
+    assert(lock.owns_lock());
+  }
+
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
   // Throws: Any exception thrown by pred.
   {

>From 63e80365294854e43d61449b9e4264d6996291bc Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Mon, 15 Jan 2024 12:08:26 +0000
Subject: [PATCH 3/4] address feedback

---
 libcxx/include/condition_variable             | 44 ++++++++++++-------
 .../thread.condition.condvarany/helpers.h     | 33 ++++++++++++++
 .../wait_for_token_pred.pass.cpp              | 16 +++++++
 .../wait_until_token_pred.pass.cpp            | 38 ++++++++++------
 4 files changed, 103 insertions(+), 28 deletions(-)
 create mode 100644 libcxx/test/std/thread/thread.condition/thread.condition.condvarany/helpers.h

diff --git a/libcxx/include/condition_variable b/libcxx/include/condition_variable
index e6399155d87edd..e375c986e7f12e 100644
--- a/libcxx/include/condition_variable
+++ b/libcxx/include/condition_variable
@@ -206,7 +206,7 @@ struct __unlock_guard {
 
   _LIBCPP_HIDE_FROM_ABI __unlock_guard(_Lock& __lock) : __lock_(__lock) { __lock_.unlock(); }
 
-  _LIBCPP_HIDE_FROM_ABI ~__unlock_guard() noexcept // turns exception to std::terminate
+  _LIBCPP_HIDE_FROM_ABI ~__unlock_guard() _NOEXCEPT // turns exception to std::terminate
   {
     __lock_.lock();
   }
@@ -263,16 +263,26 @@ condition_variable_any::wait_for(_Lock& __lock, const chrono::duration<_Rep, _Pe
 
 template <class _Lock, class _Predicate>
 bool condition_variable_any::wait(_Lock& __user_lock, stop_token __stoken, _Predicate __pred) {
-  // It is not safe to call the destructor on the other thread while waiting,
-  // even if we copy the shared_ptr<mutex> inside the function,
-  // because the stop_callback needs the internal cv.
-  // In order to make it work, we need to change the ABI to make both
-  // mutex and internal cv to be hold inside the shared_ptr, and
-  // make a local copy inside this function
-
   if (__stoken.stop_requested())
     return __pred();
-  std::stop_callback __cb(__stoken, [this] { notify_all(); });
+
+  // Per https://eel.is/c++draft/thread.condition.condvarany#general-note-2,
+  // we do need to take a copy of the shared pointer __mut_
+  // This ensures that a thread can call the destructor immediately after calling
+  // notify_all, without waiting all the wait calls.
+  // A thread can also safely call the destructor immediately after calling
+  // request_stop, as the call to request_stop would evaluate the callback,
+  // which accesses the internal condition variable, immediately on the same thread.
+  // In this situation, it is OK even without copying a shared ownership the internal
+  // condition variable. However, this needs the evaluation of stop_callback to
+  // happen-before the destruction.
+  // The spec only says "Only the notification to unblock the wait needs to happen
+  // before destruction". To make this work, we need to copy the shared ownership of
+  // the internal condition variable inside this function, which is not possible
+  // with the current ABI.
+  shared_ptr<mutex> __mut = __mut_;
+
+  stop_callback __cb(__stoken, [this] { notify_all(); });
 
   while (true) {
     if (__pred())
@@ -282,14 +292,15 @@ bool condition_variable_any::wait(_Lock& __user_lock, stop_token __stoken, _Pred
     // so that the notification cannot come in between the stop_requested
     // check and entering the wait.
     // Note that the stop_callback takes the same internal lock before notifying
-    unique_lock<mutex> __internal_lock(*__mut_);
+    unique_lock<mutex> __internal_lock(*__mut);
     if (__stoken.stop_requested())
       break;
 
     __unlock_guard<_Lock> __unlock(__user_lock);
-    unique_lock<mutex> __internal_lock2(std::move(__internal_lock));
+    unique_lock<mutex> __internal_lock2(
+        std::move(__internal_lock)); // switch unlock order between __internal_lock and __user_lock
     __cv_.wait(__internal_lock2);
-  } // __mut_.unlock(), __lock.lock()
+  } // __internal_lock2.unlock(), __user_lock.lock()
   return __pred();
 }
 
@@ -301,22 +312,25 @@ bool condition_variable_any::wait_until(
     _Predicate __pred) {
   if (__stoken.stop_requested())
     return __pred();
+
+  shared_ptr<mutex> __mut = __mut_;
   stop_callback __cb(__stoken, [this] { notify_all(); });
 
   while (true) {
     if (__pred())
       return true;
 
-    unique_lock<mutex> __internal_lock(*__mut_);
+    unique_lock<mutex> __internal_lock(*__mut);
     if (__stoken.stop_requested())
       break;
 
     __unlock_guard<_Lock> __unlock(__user_lock);
-    unique_lock<mutex> __internal_lock2(std::move(__internal_lock));
+    unique_lock<mutex> __internal_lock2(
+        std::move(__internal_lock)); // switch unlock order between __internal_lock and __user_lock
 
     if (__cv_.wait_until(__internal_lock2, __abs_time) == cv_status::timeout)
       break;
-  }
+  } // __internal_lock2.unlock(), __user_lock.lock()
   return __pred();
 }
 
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/helpers.h b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/helpers.h
new file mode 100644
index 00000000000000..4f96850773dd99
--- /dev/null
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/helpers.h
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_STD_THREAD_CONDITION_CONDVARANY_HELPERS_H
+#define TEST_STD_THREAD_CONDITION_CONDVARANY_HELPERS_H
+
+#include <chrono>
+#include <cassert>
+
+#include "test_macros.h"
+
+#if TEST_STD_VER >= 17
+
+struct ElapsedTimeCheck {
+  ElapsedTimeCheck(std::chrono::steady_clock::duration timeoutDuration)
+      : timeout_(std::chrono::steady_clock::now() + timeoutDuration) {}
+
+  ElapsedTimeCheck(ElapsedTimeCheck&&)            = delete;
+  ElapsedTimeCheck& operator=(ElapsedTimeCheck&&) = delete;
+
+  ~ElapsedTimeCheck() { assert(std::chrono::steady_clock::now() < timeout_); }
+
+  std::chrono::time_point<std::chrono::steady_clock> timeout_;
+};
+
+#endif
+
+#endif // TEST_STD_THREAD_CONDITION_CONDVARANY_HELPERS_H
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
index d06576d7c3dd09..796dd3ce12d060 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp
@@ -29,6 +29,7 @@
 #include <stop_token>
 #include <thread>
 
+#include "helpers.h"
 #include "make_test_thread.h"
 #include "test_macros.h"
 
@@ -44,6 +45,8 @@ void test() {
     Lock lock{mutex};
     ss.request_stop();
 
+    ElapsedTimeCheck check(1min);
+
     // [Note 4: The returned value indicates whether the predicate evaluated to true
     // regardless of whether the timeout was triggered or a stop request was made.]
     std::same_as<bool> auto r1 = cv.wait_for(lock, ss.get_token(), -1h, []() { return false; });
@@ -69,6 +72,8 @@ void test() {
     Mutex mutex;
     Lock lock{mutex};
 
+    ElapsedTimeCheck check(1min);
+
     std::same_as<bool> auto r1 = cv.wait_for(lock, ss.get_token(), -1h, []() { return true; });
     assert(r1);
 
@@ -83,6 +88,8 @@ void test() {
     Mutex mutex;
     Lock lock{mutex};
 
+    ElapsedTimeCheck check(1min);
+
     std::same_as<bool> auto r1 = cv.wait_for(lock, ss.get_token(), -1h, []() { return false; });
     assert(!r1);
   }
@@ -117,6 +124,8 @@ void test() {
       cv.notify_all();
     });
 
+    ElapsedTimeCheck check(10min);
+
     std::same_as<bool> auto r1 = cv.wait_for(lock, ss.get_token(), 1h, [&]() { return flag; });
     assert(flag);
     assert(r1);
@@ -143,6 +152,8 @@ void test() {
       }
     });
 
+    ElapsedTimeCheck check(10min);
+
     std::same_as<bool> auto r = cv.wait_for(lock, ss.get_token(), 1h, [&]() {
       start.store(true);
       start.notify_all();
@@ -174,6 +185,8 @@ void test() {
       std::jthread thread_;
     };
 
+    ElapsedTimeCheck check(10min);
+
     [[maybe_unused]] MyThread my_thread;
   }
 
@@ -192,6 +205,8 @@ void test() {
       request_stop_called.store(true);
     });
 
+    ElapsedTimeCheck check(10min);
+
     std::same_as<bool> auto r = cv.wait_for(lock, ss.get_token(), 1h, [&]() {
       pred_started.store(true);
       request_stop_called.wait(false);
@@ -212,6 +227,7 @@ void test() {
     Lock lock{mutex};
 
     try {
+      ElapsedTimeCheck check(10min);
       cv.wait_for(lock, ss.get_token(), 1h, []() -> bool { throw 5; });
       assert(false);
     } catch (int i) {
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
index 95371aa395f4e6..98f4354d525cf8 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
@@ -29,13 +29,14 @@
 #include <stop_token>
 #include <thread>
 
+#include "helpers.h"
 #include "make_test_thread.h"
 #include "test_macros.h"
 
 template <class Mutex, class Lock>
 void test() {
-  const auto past   = std::chrono::steady_clock::now() - std::chrono::hours(1);
-  const auto future = std::chrono::steady_clock::now() + std::chrono::hours(1);
+  const auto oneHourAgo   = std::chrono::steady_clock::now() - std::chrono::hours(1);
+  const auto oneHourLater = std::chrono::steady_clock::now() + std::chrono::hours(1);
 
   // stop_requested before hand
   {
@@ -44,19 +45,20 @@ void test() {
     Mutex mutex;
     Lock lock{mutex};
     ss.request_stop();
+    ElapsedTimeCheck check(1min);
 
     // [Note 4: The returned value indicates whether the predicate evaluated to true
     // regardless of whether the timeout was triggered or a stop request was made.]
-    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), past, []() { return false; });
+    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), oneHourAgo, []() { return false; });
     assert(!r1);
 
-    std::same_as<bool> auto r2 = cv.wait_until(lock, ss.get_token(), future, []() { return false; });
+    std::same_as<bool> auto r2 = cv.wait_until(lock, ss.get_token(), oneHourLater, []() { return false; });
     assert(!r2);
 
-    std::same_as<bool> auto r3 = cv.wait_until(lock, ss.get_token(), past, []() { return true; });
+    std::same_as<bool> auto r3 = cv.wait_until(lock, ss.get_token(), oneHourAgo, []() { return true; });
     assert(r3);
 
-    std::same_as<bool> auto r4 = cv.wait_until(lock, ss.get_token(), future, []() { return true; });
+    std::same_as<bool> auto r4 = cv.wait_until(lock, ss.get_token(), oneHourLater, []() { return true; });
     assert(r4);
 
     // Postconditions: lock is locked by the calling thread.
@@ -69,11 +71,12 @@ void test() {
     std::condition_variable_any cv;
     Mutex mutex;
     Lock lock{mutex};
+    ElapsedTimeCheck check(1min);
 
-    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), past, []() { return true; });
+    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), oneHourAgo, []() { return true; });
     assert(r1);
 
-    std::same_as<bool> auto r2 = cv.wait_until(lock, ss.get_token(), future, []() { return true; });
+    std::same_as<bool> auto r2 = cv.wait_until(lock, ss.get_token(), oneHourLater, []() { return true; });
     assert(r2);
   }
 
@@ -83,8 +86,9 @@ void test() {
     std::condition_variable_any cv;
     Mutex mutex;
     Lock lock{mutex};
+    ElapsedTimeCheck check(1min);
 
-    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), past, []() { return false; });
+    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), oneHourAgo, []() { return false; });
     assert(!r1);
   }
 
@@ -119,7 +123,9 @@ void test() {
       cv.notify_all();
     });
 
-    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), future, [&]() { return flag; });
+    ElapsedTimeCheck check(10min);
+
+    std::same_as<bool> auto r1 = cv.wait_until(lock, ss.get_token(), oneHourLater, [&]() { return flag; });
     assert(flag);
     assert(r1);
 
@@ -145,7 +151,9 @@ void test() {
       }
     });
 
-    std::same_as<bool> auto r = cv.wait_until(lock, ss.get_token(), future, [&]() {
+    ElapsedTimeCheck check(10min);
+
+    std::same_as<bool> auto r = cv.wait_until(lock, ss.get_token(), oneHourLater, [&]() {
       start.store(true);
       start.notify_all();
       return false;
@@ -176,6 +184,8 @@ void test() {
       std::jthread thread_;
     };
 
+    ElapsedTimeCheck check(10min);
+
     [[maybe_unused]] MyThread my_thread;
   }
 
@@ -194,7 +204,9 @@ void test() {
       request_stop_called.store(true);
     });
 
-    std::same_as<bool> auto r = cv.wait_until(lock, ss.get_token(), future, [&]() {
+    ElapsedTimeCheck check(10min);
+
+    std::same_as<bool> auto r = cv.wait_until(lock, ss.get_token(), oneHourLater, [&]() {
       pred_started.store(true);
       request_stop_called.wait(false);
       return false;
@@ -214,7 +226,7 @@ void test() {
     Lock lock{mutex};
 
     try {
-      cv.wait_until(lock, ss.get_token(), future, []() -> bool { throw 5; });
+      cv.wait_until(lock, ss.get_token(), oneHourLater, []() -> bool { throw 5; });
       assert(false);
     } catch (int i) {
       assert(i == 5);

>From 8d9006c4b7943e9e969a148c23dad5032031c2c7 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Mon, 15 Jan 2024 13:45:13 +0000
Subject: [PATCH 4/4] ci

---
 .../wait_until_token_pred.pass.cpp                           | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
index 98f4354d525cf8..014e7f3f7a41d9 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp
@@ -35,8 +35,9 @@
 
 template <class Mutex, class Lock>
 void test() {
-  const auto oneHourAgo   = std::chrono::steady_clock::now() - std::chrono::hours(1);
-  const auto oneHourLater = std::chrono::steady_clock::now() + std::chrono::hours(1);
+  using namespace std::chrono_literals;
+  const auto oneHourAgo   = std::chrono::steady_clock::now() - 1h;
+  const auto oneHourLater = std::chrono::steady_clock::now() + 1h;
 
   // stop_requested before hand
   {



More information about the libcxx-commits mailing list