[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