[libc++] fix condition_variable_any hangs on stop_request

[huixie90]

When I implemented condition_variable_any::wait, I missed the most important paragraph in the spec:

The following wait functions will be notified when there is a stop request on the passed stop_token.
In that case the functions return immediately, returning false if the predicate evaluates to false.

From https://eel.is/c++draft/thread.condition#thread.condvarany.intwait-1.

Fixes #76807

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/77127.diff

4 Files Affected:

• (modified) libcxx/include/condition_variable (+3)
• (modified) libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp (+22)
• (modified) libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp (+22)
• (modified) libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp (+22)

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.
   {

[alexbatashev]

Thank you very much for taking time and fixing the issue!

[sn-rbroker]

This PR looks good to me, I checked through the new test cases and they seem like they cover the hang issues noted already, and the other improvements (using std::move for the internal lock, renaming the user lock, checking stop_requested/predicate before taking the time to create the stop callback) all look good.

(I guess the test failure around noexcept -> _NOEXCEPT with c++03 is something you're already looking at.)

Thanks @huixie90!

[ldionne]

LGTM!

[ldionne]

No, this was merged in time and the commit (85a8e5c) is on release/18.x.

Edit: Ugh, I meant to post this to the issue as an answer to Tom's question.