Implement LWG-4301: condition_variable{_any}::wait_{for, until} should take timeout by value

stl/inc/condition_variable

@@ -90,7 +90,7 @@ public:
     }
 
     template <class _Lock, class _Clock, class _Duration>
-    cv_status wait_until(_Lock& _Lck, const chrono::time_point<_Clock, _Duration>& _Abs_time) {
+    cv_status wait_until(_Lock& _Lck, const chrono::time_point<_Clock, _Duration> _Abs_time) {
         // wait until time point
         static_assert(chrono::_Is_clock_v<_Clock>, "Clock type required");
         const auto _Now                  = _Clock::now();
@@ -100,7 +100,7 @@ public:
     }
 
     template <class _Lock, class _Clock, class _Duration, class _Predicate>
-    bool wait_until(_Lock& _Lck, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) {
+    bool wait_until(_Lock& _Lck, const chrono::time_point<_Clock, _Duration> _Abs_time, _Predicate _Pred) {
         // wait for signal with timeout and check predicate
 #if _HAS_CXX20
         static_assert(chrono::is_clock_v<_Clock>, "Clock type required");
@@ -115,7 +115,7 @@ public:
     }
 
     template <class _Lock, class _Rep, class _Period>
-    cv_status wait_for(_Lock& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time) { // wait for duration
+    cv_status wait_for(_Lock& _Lck, const chrono::duration<_Rep, _Period> _Rel_time) { // wait for duration
         if (_Rel_time <= chrono::duration<_Rep, _Period>::zero()) {
             _Unlock_guard<_Lock> _Unlock_outer{_Lck};
             (void) _Unlock_outer;
@@ -146,7 +146,7 @@ public:
     }
 
     template <class _Lock, class _Rep, class _Period, class _Predicate>
-    bool wait_for(_Lock& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time, _Predicate _Pred) {
+    bool wait_for(_Lock& _Lck, const chrono::duration<_Rep, _Period> _Rel_time, _Predicate _Pred) {
         // wait for signal with timeout and check predicate
         return wait_until(_Lck, _To_absolute_time(_Rel_time), _STD move(_Pred));
     }
@@ -194,7 +194,7 @@ public:
 
     template <class _Lock, class _Clock, class _Duration, class _Predicate>
     bool wait_until(
-        _Lock& _Lck, stop_token _Stoken, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) {
+        _Lock& _Lck, stop_token _Stoken, const chrono::time_point<_Clock, _Duration> _Abs_time, _Predicate _Pred) {
         static_assert(chrono::is_clock_v<_Clock>, "Clock type required");
         stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this};
         for (;;) {
@@ -224,7 +224,7 @@ public:
     }
 
     template <class _Lock, class _Rep, class _Period, class _Predicate>
-    bool wait_for(_Lock& _Lck, stop_token _Stoken, const chrono::duration<_Rep, _Period>& _Rel_time, _Predicate _Pred) {
+    bool wait_for(_Lock& _Lck, stop_token _Stoken, const chrono::duration<_Rep, _Period> _Rel_time, _Predicate _Pred) {
         return wait_until(_Lck, _STD move(_Stoken), _To_absolute_time(_Rel_time), _STD move(_Pred));
     }
 #endif // _HAS_CXX20

stl/inc/mutex

@@ -556,7 +556,7 @@ public:
     }
 
     template <class _Rep, class _Period>
-    cv_status wait_for(unique_lock<mutex>& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time) {
+    cv_status wait_for(unique_lock<mutex>& _Lck, const chrono::duration<_Rep, _Period> _Rel_time) {
         // wait for duration
         if (_Rel_time <= chrono::duration<_Rep, _Period>::zero()) {
             // we don't unlock-and-relock _Lck for this case because it's not observable
@@ -566,13 +566,13 @@ public:
     }
 
     template <class _Rep, class _Period, class _Predicate>
-    bool wait_for(unique_lock<mutex>& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time, _Predicate _Pred) {
+    bool wait_for(unique_lock<mutex>& _Lck, const chrono::duration<_Rep, _Period> _Rel_time, _Predicate _Pred) {
         // wait for signal with timeout and check predicate
         return wait_until(_Lck, _To_absolute_time(_Rel_time), _STD _Pass_fn(_Pred));
     }
 
     template <class _Clock, class _Duration>
-    cv_status wait_until(unique_lock<mutex>& _Lck, const chrono::time_point<_Clock, _Duration>& _Abs_time) {
+    cv_status wait_until(unique_lock<mutex>& _Lck, const chrono::time_point<_Clock, _Duration> _Abs_time) {
         // wait until time point
         static_assert(chrono::_Is_clock_v<_Clock>, "Clock type required");
 #if _ITERATOR_DEBUG_LEVEL != 0
@@ -598,8 +598,7 @@ public:
     }
 
     template <class _Clock, class _Duration, class _Predicate>
-    bool wait_until(
-        unique_lock<mutex>& _Lck, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) {
+    bool wait_until(unique_lock<mutex>& _Lck, const chrono::time_point<_Clock, _Duration> _Abs_time, _Predicate _Pred) {
         // wait for signal with timeout and check predicate
         static_assert(chrono::_Is_clock_v<_Clock>, "Clock type required");
         while (!_Pred()) {

stl/inc/yvals_core.h

@@ -84,6 +84,7 @@
 // P3323R1 Forbid atomic<cv T>, Specify atomic_ref<cv T>
 //     (for atomic<cv T>)
 // P3503R3 Make Type-Erased Allocator Use In promise And packaged_task Consistent
+// LWG-4301 condition_variable{_any}::wait_{for, until} should take timeout by value
 
 // _HAS_CXX17 directly controls:
 // P0005R4 not_fn()

tests/std/tests/GH_000685_condition_variable_any/test.cpp

@@ -2,11 +2,13 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 // Test GH-685 "wait_for in condition_variable_any should unlock and lock"
+// Test LWG-4301 "condition_variable{_any}::wait_{for, until} should take timeout by value"
 
 #include <cassert>
 #include <chrono>
 #include <condition_variable>
 #include <mutex>
+#include <thread>
 #include <type_traits>
 
 using namespace std;
@@ -79,9 +81,96 @@ namespace {
         assert(m.num_locks() == 4);
 #endif // _HAS_CXX20
     }
+
+    // Minimal example inspired by LWG-4301, modified due to missing latch in C++11 to 20
+    // and generalized to test all overloads of condition_variable{_any}::wait_{for, until}
+    template <typename CV>
+    void test_timeout_immutable(int test_index) {
+
+        mutex m;
+        CV cv;
+        unique_lock<mutex> lock(m); // Ensure main thread waitins on cv before other_thread starts
+
+        // Start with very short timeout and let other_thread change it to very large while main thread is waiting
+        constexpr auto short_timeout = 100ms;
+        constexpr auto long_timeout  = 2s;
+
+        auto timeout_duration = short_timeout;
+        auto timeout          = steady_clock::now() + timeout_duration;
+
+        thread other_thread([&] {
+            // Immediatelly blocks since the main thread owns lock.
+            unique_lock<mutex> lock(m);
+
+            // If the timeout provided to condition_variable{_any}::wait_{for, until} was mutable,
+            // we will get timeout after much longer time
+            timeout_duration = long_timeout;
+            timeout          = steady_clock::now() + timeout_duration;
+        });
+
+        const auto wait_start = steady_clock::now();
+#define CHECK_TIMEOUT_NOT_CHANGED() assert(steady_clock::now() - wait_start < long_timeout / 2);
+
+        switch (test_index) {
+        case 0:
+            assert(cv.wait_until(lock, timeout) == cv_status::timeout);
+            CHECK_TIMEOUT_NOT_CHANGED();
+            break;
+
+        case 1:
+            assert(cv.wait_until(lock, timeout, [] { return false; }) == false);
+            CHECK_TIMEOUT_NOT_CHANGED();
+            break;
+
+        case 2:
+            assert(cv.wait_for(lock, timeout_duration) == cv_status::timeout);
+            CHECK_TIMEOUT_NOT_CHANGED();
+            break;
+
+        case 3:
+            assert(cv.wait_for(lock, timeout_duration, [] { return false; }) == false);
+            CHECK_TIMEOUT_NOT_CHANGED();
+            break;
+
+#if _HAS_CXX20 // because of stop_token
+        case 4:
+            if constexpr (std::is_same_v<CV, std::condition_variable_any>) {
+                stop_source source;
+                assert(cv.wait_until(lock, source.get_token(), timeout, [] { return false; }) == false);
+                CHECK_TIMEOUT_NOT_CHANGED();
+            } else {
+                assert(false);
+            }
+            break;
+
+        case 5:
+            if constexpr (std::is_same_v<CV, std::condition_variable_any>) {
+                stop_source source;
+                assert(cv.wait_for(lock, source.get_token(), timeout_duration, [] { return false; }) == false);
+                CHECK_TIMEOUT_NOT_CHANGED();
+            } else {
+                assert(false);
+            }
+            break;
+#endif // _HAS_CXX20
+
+        default:
+            assert(false);
+        }
+
+        other_thread.join();
+    }
 } // unnamed namespace
 
 int main() {
     test_condition_variable_any();
     test_condition_variable_any_already_timed_out();
+
+    for (int i = 0; i < 4; i++) {
+        test_timeout_immutable<std::condition_variable>(i);
+    }
+
+    for (int i = 0; i < (_HAS_CXX20 ? 6 : 4); i++) {
+        test_timeout_immutable<std::condition_variable_any>(i);
+    }
 }