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

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Jan 12 09:22:33 PST 2024


================
@@ -256,23 +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) {
-  while (!__stoken.stop_requested()) {
+bool condition_variable_any::wait(_Lock& __user_lock, stop_token __stoken, _Predicate __pred) {
----------------
ldionne wrote:

Per https://eel.is/c++draft/thread.condition.condvarany#general-note-2, it looks like we do need to take a copy of the shared pointer `__mut_`. We would also technically need to make a local copy of the condition variable, but I think we can't do that without an ABI break. We can either simply not do it (with a comment), or do it conditionally on an ABI macro.

I think doing the ABI macro thing is worthwhile, but it should be done in a separate patch because it's going to make this patch too complicated. Idea from our live review:

```diff
diff --git a/libcxx/include/condition_variable b/libcxx/include/condition_variable
index cf7a570b6cb6..2c7ec1ca1e54 100644
--- a/libcxx/include/condition_variable
+++ b/libcxx/include/condition_variable
@@ -144,9 +144,29 @@ public:
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 class _LIBCPP_EXPORTED_FROM_ABI condition_variable_any {
+#ifdef _LIBCPP_ABI_CONDITION_VARIABLE_ANY_FIX_FOO
+  struct __internal_state {
+    condition_variable __cv_;
+    mutex __mut_;
+  };
+  shared_ptr<__internal_state> __x;
+
+  condition_variable& __get_internal_cv() { return __x->__cv_; }
+  mutex& __get_internal_mutex() { return __x->__mut_; }
+  shared_ptr<__internal_state> __save_internal_state() { return __x; }
+#else
   condition_variable __cv_;
   shared_ptr<mutex> __mut_;
 
+  condition_variable& __get_internal_cv() { return __cv_; }
+  mutex& __get_internal_mutex() { return __mut_; }
+  shared_ptr<mutex> __save_internal_state() { return __mut_; }
+#endif
+
+  // then in the wait() functions you can do something like:
+  auto __saved = __save_internal_state();
+  just_keep_using(__mut_); // you can't actually do this but that's the spirit
+
 public:
   _LIBCPP_HIDE_FROM_ABI condition_variable_any();
 

```

The ABI macro would be defined in `__config`, look for e.g. below `#  if _LIBCPP_ABI_VERSION >= 2`.

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


More information about the libcxx-commits mailing list