[libcxx-commits] [libcxx] [libc++] Save duration/timeout locally for condition_variable waits (PR #148330)
John Sheu via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jul 11 19:44:45 PDT 2025
https://github.com/johnsheu created https://github.com/llvm/llvm-project/pull/148330
wait_for() and wait_until() for condition_variable and condition_variable_any take their wait duration or timeout parameters by const-reference. This can lead to unexpected behavior if the referent changes while the wait_for() or wait_until() blocks.
For condition_variable_any, this is an actual data race if the timeout parameter is protected by the lock, as the implementation accesses the timeout outside of the lock.
Fix this by saving a local copy of the duration or timeout while the condition variable is waiting.
>From f1dd32bc03e878cad4b941b82fe4990294428a23 Mon Sep 17 00:00:00 2001
From: John Sheu <john.sheu at gmail.com>
Date: Fri, 11 Jul 2025 17:47:37 -0700
Subject: [PATCH] [libc++] Save duration/timeout locally for condition_variable
waits
wait_for() and wait_until() for condition_variable and
condition_variable_any take their wait duration or timeout parameters
by const-reference. This can lead to unexpected behavior if the
referent changes while the wait_for() or wait_until() blocks.
For condition_variable_any, this is an actual data race if the timeout
parameter is protected by the lock, as the implementation accesses the
timeout outside of the lock.
Fix this by saving a local copy of the duration or timeout while the
condition variable is waiting.
---
.../__condition_variable/condition_variable.h | 17 ++++++++++-------
libcxx/include/condition_variable | 9 ++++++---
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/libcxx/include/__condition_variable/condition_variable.h b/libcxx/include/__condition_variable/condition_variable.h
index 1e8edd5dcb009..07f81c532127a 100644
--- a/libcxx/include/__condition_variable/condition_variable.h
+++ b/libcxx/include/__condition_variable/condition_variable.h
@@ -118,21 +118,23 @@ class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
using namespace chrono;
using __clock_tp_ns = time_point<_Clock, nanoseconds>;
- typename _Clock::time_point __now = _Clock::now();
+ const typename _Clock::time_point __now = _Clock::now();
if (__t <= __now)
return cv_status::timeout;
+ const typename _Clock::time_point __t_local = __t;
- __clock_tp_ns __t_ns = __clock_tp_ns(std::__safe_nanosecond_cast(__t.time_since_epoch()));
+ __clock_tp_ns __t_ns = __clock_tp_ns(std::__safe_nanosecond_cast(__t_local.time_since_epoch()));
__do_timed_wait(__lk, __t_ns);
- return _Clock::now() < __t ? cv_status::no_timeout : cv_status::timeout;
+ return _Clock::now() < __t_local ? cv_status::no_timeout : cv_status::timeout;
}
template <class _Clock, class _Duration, class _Predicate>
_LIBCPP_HIDE_FROM_ABI bool
wait_until(unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t, _Predicate __pred) {
+ const typename _Clock::time_point __t_local = __t;
while (!__pred()) {
- if (wait_until(__lk, __t) == cv_status::timeout)
+ if (wait_until(__lk, __t_local) == cv_status::timeout)
return __pred();
}
return true;
@@ -144,7 +146,8 @@ class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
if (__d <= __d.zero())
return cv_status::timeout;
using __ns_rep = nanoseconds::rep;
- steady_clock::time_point __c_now = steady_clock::now();
+ const steady_clock::time_point __c_now = steady_clock::now();
+ const duration<_Rep, _Period> __d_local = __d;
# if _LIBCPP_HAS_COND_CLOCKWAIT
using __clock_tp_ns = time_point<steady_clock, nanoseconds>;
@@ -154,7 +157,7 @@ class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
__ns_rep __now_count_ns = std::__safe_nanosecond_cast(system_clock::now().time_since_epoch()).count();
# endif
- __ns_rep __d_ns_count = std::__safe_nanosecond_cast(__d).count();
+ __ns_rep __d_ns_count = std::__safe_nanosecond_cast(__d_local).count();
if (__now_count_ns > numeric_limits<__ns_rep>::max() - __d_ns_count) {
__do_timed_wait(__lk, __clock_tp_ns::max());
@@ -162,7 +165,7 @@ class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
__do_timed_wait(__lk, __clock_tp_ns(nanoseconds(__now_count_ns + __d_ns_count)));
}
- return steady_clock::now() - __c_now < __d ? cv_status::no_timeout : cv_status::timeout;
+ return steady_clock::now() - __c_now < __d_local ? cv_status::no_timeout : cv_status::timeout;
}
template <class _Rep, class _Period, class _Predicate>
diff --git a/libcxx/include/condition_variable b/libcxx/include/condition_variable
index 99c74b02807ae..57e995f5224b1 100644
--- a/libcxx/include/condition_variable
+++ b/libcxx/include/condition_variable
@@ -188,9 +188,10 @@ public:
_LIBCPP_HIDE_FROM_ABI cv_status wait_until(_Lock& __lock, const chrono::time_point<_Clock, _Duration>& __t) {
shared_ptr<mutex> __mut = __mut_;
unique_lock<mutex> __lk(*__mut);
+ const chrono::time_point<_Clock, _Duration> __t_local = __t;
__unlock_guard<_Lock> __unlock(__lock);
lock_guard<unique_lock<mutex> > __lx(__lk, adopt_lock_t());
- return __cv_.wait_until(__lk, __t);
+ return __cv_.wait_until(__lk, __t_local);
} // __mut_.unlock(), __lock.lock()
template <class _Lock, class _Clock, class _Duration, class _Predicate>
@@ -240,8 +241,9 @@ inline void condition_variable_any::wait(_Lock& __lock, _Predicate __pred) {
template <class _Lock, class _Clock, class _Duration, class _Predicate>
inline bool
condition_variable_any::wait_until(_Lock& __lock, const chrono::time_point<_Clock, _Duration>& __t, _Predicate __pred) {
+ const chrono::time_point<_Clock, _Duration> __t_local = __t;
while (!__pred())
- if (wait_until(__lock, __t) == cv_status::timeout)
+ if (wait_until(__lock, __t_local) == cv_status::timeout)
return __pred();
return true;
}
@@ -313,6 +315,7 @@ bool condition_variable_any::wait_until(
shared_ptr<mutex> __mut = __mut_;
stop_callback __cb(__stoken, [this] { notify_all(); });
+ const chrono::time_point<_Clock, _Duration> __abs_time_local = __abs_time;
while (true) {
if (__pred())
@@ -326,7 +329,7 @@ bool condition_variable_any::wait_until(
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)
+ if (__cv_.wait_until(__internal_lock2, __abs_time_local) == cv_status::timeout)
break;
} // __internal_lock2.unlock(), __user_lock.lock()
return __pred();
More information about the libcxx-commits
mailing list