From 790759e1fa53cdf14093c910430affd2df7adb8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Michal?= Date: Thu, 20 Nov 2025 01:49:16 +0100 Subject: [PATCH 01/14] Provide tests of immutability of timeout/duration passed to conditional_variable{_any}::wait_{for, until} --- .../GH_000685_condition_variable_any/test.cpp | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index 03ebd7023b8..597823550f1 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/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 #include #include #include +#include #include 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 + void test_timeout_immutable(int test_index) { + + mutex m; + CV cv; + unique_lock 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 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) { + 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) { + 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(i); + } + + for (int i = 0; i < (_HAS_CXX20 ? 6 : 4); i++) { + test_timeout_immutable(i); + } } From a517c8bf5dfcd1aff1c367c6b9477cfac57ff3b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Michal?= Date: Thu, 20 Nov 2025 01:49:58 +0100 Subject: [PATCH 02/14] Pass timeout to conditional_variable{_any}::wait_{for, until} by value. --- stl/inc/condition_variable | 12 ++++++------ stl/inc/mutex | 9 ++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 8d52f7265bf..44d381e4150 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -90,7 +90,7 @@ public: } template - 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 - 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 - 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 - 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 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 - 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 diff --git a/stl/inc/mutex b/stl/inc/mutex index 8cb450c59d7..0deda247d62 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -556,7 +556,7 @@ public: } template - cv_status wait_for(unique_lock& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time) { + cv_status wait_for(unique_lock& _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 - bool wait_for(unique_lock& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time, _Predicate _Pred) { + bool wait_for(unique_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 _Pass_fn(_Pred)); } template - cv_status wait_until(unique_lock& _Lck, const chrono::time_point<_Clock, _Duration>& _Abs_time) { + cv_status wait_until(unique_lock& _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 - bool wait_until( - unique_lock& _Lck, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) { + bool wait_until(unique_lock& _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()) { From 51ab2869d10cb85e59c2957b54c06b1e4cfcc521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Michal?= Date: Thu, 20 Nov 2025 01:52:55 +0100 Subject: [PATCH 03/14] Mention LWG-4301 as unconditionally implemented in yvals_core.h --- stl/inc/yvals_core.h | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index d8f8708946e..306d91537d4 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -84,6 +84,7 @@ // P3323R1 Forbid atomic, Specify atomic_ref // (for atomic) // 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() From cd8de92edc4dc9f4b9aa199764fb4b0447564408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Michal?= Date: Thu, 20 Nov 2025 09:46:33 +0100 Subject: [PATCH 04/14] Fix review comments: Unroll invocation of tests, remove mention of LWG-4301 from yvals_core.h --- stl/inc/yvals_core.h | 1 - .../GH_000685_condition_variable_any/test.cpp | 22 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 306d91537d4..d8f8708946e 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -84,7 +84,6 @@ // P3323R1 Forbid atomic, Specify atomic_ref // (for atomic) // 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() diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index 597823550f1..980f9f742f1 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -85,7 +85,7 @@ namespace { // 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 - void test_timeout_immutable(int test_index) { + void test_timeout_immutable(int test_number) { mutex m; CV cv; @@ -111,7 +111,7 @@ namespace { const auto wait_start = steady_clock::now(); #define CHECK_TIMEOUT_NOT_CHANGED() assert(steady_clock::now() - wait_start < long_timeout / 2); - switch (test_index) { + switch (test_number) { case 0: assert(cv.wait_until(lock, timeout) == cv_status::timeout); CHECK_TIMEOUT_NOT_CHANGED(); @@ -166,11 +166,17 @@ int main() { test_condition_variable_any(); test_condition_variable_any_already_timed_out(); - for (int i = 0; i < 4; i++) { - test_timeout_immutable(i); - } + test_timeout_immutable(0); + test_timeout_immutable(1); + test_timeout_immutable(2); + test_timeout_immutable(3); - for (int i = 0; i < (_HAS_CXX20 ? 6 : 4); i++) { - test_timeout_immutable(i); - } + test_timeout_immutable(0); + test_timeout_immutable(1); + test_timeout_immutable(2); + test_timeout_immutable(3); +#if _HAS_CXX20 + test_timeout_immutable(4); + test_timeout_immutable(5); +#endif // _HAS_CXX20 } From 727d997b4b689c77f77120c6dc85717bdbd5facf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Michal?= Date: Fri, 21 Nov 2025 07:06:44 +0100 Subject: [PATCH 05/14] Properly synchronize threads in tests to minimize timing assumptions. --- .../GH_000685_condition_variable_any/test.cpp | 123 +++++++++++------- 1 file changed, 79 insertions(+), 44 deletions(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index 980f9f742f1..d00fb563324 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -4,6 +4,7 @@ // 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 #include #include #include @@ -84,80 +85,111 @@ namespace { // 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} + // Idea: Make the main thread wait for a CV with a short timeout and modify it from another thread in the meantime. + // If the main thread wait times out after short time, the modification did not influence the ongoing wait template - void test_timeout_immutable(int test_number) { + void test_timeout_immutable(int test_number, int retries_remaining = 5) { + printf("\ntest %d\n", test_number); mutex m; CV cv; - unique_lock lock(m); // Ensure main thread waitins on cv before other_thread starts + unique_lock lock(m); // Prevent other thread from modifying timeout too early // 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; + constexpr auto short_timeout = 1s; + constexpr auto long_timeout = 10s; + + atomic_flag waiting_for_other_thread = ATOMIC_FLAG_INIT; + waiting_for_other_thread.test_and_set(); auto timeout_duration = short_timeout; auto timeout = steady_clock::now() + timeout_duration; + auto const set_timeout = [&](auto const new_timeout) { + timeout_duration = new_timeout; + timeout = steady_clock::now() + new_timeout; + }; + + const auto wait_start = steady_clock::now(); + thread other_thread([&] { + printf( + "thread start after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); + waiting_for_other_thread.clear(); // Immediatelly blocks since the main thread owns lock. - unique_lock lock(m); + lock_guard lock(m); + puts("thread lock"); // 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; + // we will get timeout in the main thread after much longer time + set_timeout(long_timeout); + puts("thread end"); }); - const auto wait_start = steady_clock::now(); -#define CHECK_TIMEOUT_NOT_CHANGED() assert(steady_clock::now() - wait_start < long_timeout / 2); + while (waiting_for_other_thread.test_and_set()) { + this_thread::yield(); // freeze the main thread from proceeding until other thread is started + } + printf("main resumed after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); + set_timeout(short_timeout); - switch (test_number) { - 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; + puts("main waiting"); + const bool cv_wait_timed_out = [&] { + switch (test_number) { + case 0: + return cv.wait_until(lock, timeout) == cv_status::timeout; + case 1: + return cv.wait_until(lock, timeout, [] { return false; }) == false; - case 2: - assert(cv.wait_for(lock, timeout_duration) == cv_status::timeout); - CHECK_TIMEOUT_NOT_CHANGED(); - break; + case 2: + return cv.wait_for(lock, timeout_duration) == cv_status::timeout; - case 3: - assert(cv.wait_for(lock, timeout_duration, [] { return false; }) == false); - CHECK_TIMEOUT_NOT_CHANGED(); - break; + case 3: + return cv.wait_for(lock, timeout_duration, [] { return false; }) == false; #if _HAS_CXX20 // because of stop_token - case 4: - if constexpr (std::is_same_v) { - stop_source source; - assert(cv.wait_until(lock, source.get_token(), timeout, [] { return false; }) == false); - CHECK_TIMEOUT_NOT_CHANGED(); - } else { + case 4: + if constexpr (std::is_same_v) { + stop_source source; + return cv.wait_until(lock, source.get_token(), timeout, [] { return false; }) == false; + } else { + assert(false); // test not supported for std::condition_variable + return false; + } + + case 5: + if constexpr (std::is_same_v) { + stop_source source; + return cv.wait_for(lock, source.get_token(), timeout_duration, [] { return false; }) == false; + } else { + assert(false); // test not supported for std::condition_variable + return false; + } +#endif // _HAS_CXX20 + + default: assert(false); + return false; } - break; + }(); - case 5: - if constexpr (std::is_same_v) { - stop_source source; - assert(cv.wait_for(lock, source.get_token(), timeout_duration, [] { return false; }) == false); - CHECK_TIMEOUT_NOT_CHANGED(); + if (!cv_wait_timed_out) { + if (retries_remaining > 0) { + printf("unexpected wakeup after %lld ms, retry %d...\n", + duration_cast(steady_clock::now() - wait_start).count(), retries_remaining); + test_timeout_immutable(test_number, retries_remaining - 1); /* recurse to try the test again */ } else { + puts("Too many unexpected wakeups"); assert(false); } - break; -#endif // _HAS_CXX20 - - default: - assert(false); + } else { + assert(steady_clock::now() - wait_start < long_timeout / 2); + printf("wait end after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); } + + // Make sure the child thread has indeed finished (so the next join does not block + assert(timeout_duration == long_timeout); other_thread.join(); } } // unnamed namespace @@ -166,11 +198,14 @@ int main() { test_condition_variable_any(); test_condition_variable_any_already_timed_out(); + puts("condition_variable"); test_timeout_immutable(0); test_timeout_immutable(1); test_timeout_immutable(2); test_timeout_immutable(3); + puts("condition_variable_any"); + test_timeout_immutable(0); test_timeout_immutable(1); test_timeout_immutable(2); From 73658d678d1f68aaef2afc1a0f19fd8fad11e2e7 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:17:47 -0800 Subject: [PATCH 06/14] Improve comments. --- .../tests/GH_000685_condition_variable_any/test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index d00fb563324..73f4716898a 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -83,10 +83,10 @@ namespace { #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} + // Minimal example inspired by LWG-4301, modified due to missing std::latch before C++20 + // and generalized to test all overloads of condition_variable{_any}::wait_{for, until}. // Idea: Make the main thread wait for a CV with a short timeout and modify it from another thread in the meantime. - // If the main thread wait times out after short time, the modification did not influence the ongoing wait + // If the main thread wait times out after a short time, the modification did not influence the ongoing wait. template void test_timeout_immutable(int test_number, int retries_remaining = 5) { printf("\ntest %d\n", test_number); @@ -116,7 +116,7 @@ namespace { printf( "thread start after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); waiting_for_other_thread.clear(); - // Immediatelly blocks since the main thread owns lock. + // Immediately blocks since the main thread owns lock. lock_guard lock(m); puts("thread lock"); @@ -177,7 +177,7 @@ namespace { if (retries_remaining > 0) { printf("unexpected wakeup after %lld ms, retry %d...\n", duration_cast(steady_clock::now() - wait_start).count(), retries_remaining); - test_timeout_immutable(test_number, retries_remaining - 1); /* recurse to try the test again */ + test_timeout_immutable(test_number, retries_remaining - 1); // recurse to try the test again } else { puts("Too many unexpected wakeups"); assert(false); @@ -188,7 +188,7 @@ namespace { } - // Make sure the child thread has indeed finished (so the next join does not block + // Make sure the child thread has indeed finished (so the next join does not block) assert(timeout_duration == long_timeout); other_thread.join(); } From 272929cc6a54a32b2aaa30e0dc0e15baa4a4eb72 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:19:15 -0800 Subject: [PATCH 07/14] Include `` and ``. --- tests/std/tests/GH_000685_condition_variable_any/test.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index 73f4716898a..ada6e445d04 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -8,10 +8,15 @@ #include #include #include +#include #include #include #include +#if _HAS_CXX20 +#include +#endif // _HAS_CXX20 + using namespace std; using namespace std::chrono; From a742482133c698021bf654c97f7fdd80a8a3b2da Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:21:15 -0800 Subject: [PATCH 08/14] All shall love `using namespace std;` and despair. --- .../GH_000685_condition_variable_any/test.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index ada6e445d04..ba435f6cfd5 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -154,7 +154,7 @@ namespace { #if _HAS_CXX20 // because of stop_token case 4: - if constexpr (std::is_same_v) { + if constexpr (is_same_v) { stop_source source; return cv.wait_until(lock, source.get_token(), timeout, [] { return false; }) == false; } else { @@ -163,7 +163,7 @@ namespace { } case 5: - if constexpr (std::is_same_v) { + if constexpr (is_same_v) { stop_source source; return cv.wait_for(lock, source.get_token(), timeout_duration, [] { return false; }) == false; } else { @@ -204,19 +204,19 @@ int main() { test_condition_variable_any_already_timed_out(); puts("condition_variable"); - test_timeout_immutable(0); - test_timeout_immutable(1); - test_timeout_immutable(2); - test_timeout_immutable(3); + test_timeout_immutable(0); + test_timeout_immutable(1); + test_timeout_immutable(2); + test_timeout_immutable(3); puts("condition_variable_any"); - test_timeout_immutable(0); - test_timeout_immutable(1); - test_timeout_immutable(2); - test_timeout_immutable(3); + test_timeout_immutable(0); + test_timeout_immutable(1); + test_timeout_immutable(2); + test_timeout_immutable(3); #if _HAS_CXX20 - test_timeout_immutable(4); - test_timeout_immutable(5); + test_timeout_immutable(4); + test_timeout_immutable(5); #endif // _HAS_CXX20 } From 361c4c4602922623c3fda6dda5f2fe52a2ab920c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:23:09 -0800 Subject: [PATCH 09/14] Adjust newlines. --- tests/std/tests/GH_000685_condition_variable_any/test.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index ba435f6cfd5..d36f1155730 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -137,12 +137,12 @@ namespace { printf("main resumed after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); set_timeout(short_timeout); - puts("main waiting"); const bool cv_wait_timed_out = [&] { switch (test_number) { case 0: return cv.wait_until(lock, timeout) == cv_status::timeout; + case 1: return cv.wait_until(lock, timeout, [] { return false; }) == false; @@ -192,7 +192,6 @@ namespace { printf("wait end after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); } - // Make sure the child thread has indeed finished (so the next join does not block) assert(timeout_duration == long_timeout); other_thread.join(); @@ -210,7 +209,6 @@ int main() { test_timeout_immutable(3); puts("condition_variable_any"); - test_timeout_immutable(0); test_timeout_immutable(1); test_timeout_immutable(2); From 28962980277155fe1ea9c0a88281027531e09dd1 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:26:35 -0800 Subject: [PATCH 10/14] Avoid ATOMIC_FLAG_INIT. --- tests/std/tests/GH_000685_condition_variable_any/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index d36f1155730..6bcd7ac87fb 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -104,7 +104,7 @@ namespace { constexpr auto short_timeout = 1s; constexpr auto long_timeout = 10s; - atomic_flag waiting_for_other_thread = ATOMIC_FLAG_INIT; + atomic_flag waiting_for_other_thread{}; waiting_for_other_thread.test_and_set(); auto timeout_duration = short_timeout; From 272a3f13f65701d82a51e2398a5a01aafc1ae8ef Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:34:03 -0800 Subject: [PATCH 11/14] Const value parameters. --- tests/std/tests/GH_000685_condition_variable_any/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index 6bcd7ac87fb..eb8eb9822a6 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -93,7 +93,7 @@ namespace { // Idea: Make the main thread wait for a CV with a short timeout and modify it from another thread in the meantime. // If the main thread wait times out after a short time, the modification did not influence the ongoing wait. template - void test_timeout_immutable(int test_number, int retries_remaining = 5) { + void test_timeout_immutable(const int test_number, const int retries_remaining = 5) { printf("\ntest %d\n", test_number); mutex m; From 5b0b4488054331739c246f68e74616ec9efdff9c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:36:01 -0800 Subject: [PATCH 12/14] West const. --- tests/std/tests/GH_000685_condition_variable_any/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index eb8eb9822a6..575624e8755 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -110,7 +110,7 @@ namespace { auto timeout_duration = short_timeout; auto timeout = steady_clock::now() + timeout_duration; - auto const set_timeout = [&](auto const new_timeout) { + const auto set_timeout = [&](const auto new_timeout) { timeout_duration = new_timeout; timeout = steady_clock::now() + new_timeout; }; From 7f734fdaf8e9c4340bd182d5be2077aab9e307e6 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:45:17 -0800 Subject: [PATCH 13/14] Rename to main_lock and other_lock to avoid shadowing. --- .../GH_000685_condition_variable_any/test.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index 575624e8755..a75008d4e8d 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -98,7 +98,7 @@ namespace { mutex m; CV cv; - unique_lock lock(m); // Prevent other thread from modifying timeout too early + unique_lock main_lock(m); // Prevent other thread from modifying timeout too early // Start with very short timeout and let other_thread change it to very large while main thread is waiting constexpr auto short_timeout = 1s; @@ -121,8 +121,8 @@ namespace { printf( "thread start after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); waiting_for_other_thread.clear(); - // Immediately blocks since the main thread owns lock. - lock_guard lock(m); + // Immediately blocks since the main thread owns the mutex m. + lock_guard other_lock(m); puts("thread lock"); // If the timeout provided to condition_variable{_any}::wait_{for, until} was mutable, @@ -141,22 +141,22 @@ namespace { const bool cv_wait_timed_out = [&] { switch (test_number) { case 0: - return cv.wait_until(lock, timeout) == cv_status::timeout; + return cv.wait_until(main_lock, timeout) == cv_status::timeout; case 1: - return cv.wait_until(lock, timeout, [] { return false; }) == false; + return cv.wait_until(main_lock, timeout, [] { return false; }) == false; case 2: - return cv.wait_for(lock, timeout_duration) == cv_status::timeout; + return cv.wait_for(main_lock, timeout_duration) == cv_status::timeout; case 3: - return cv.wait_for(lock, timeout_duration, [] { return false; }) == false; + return cv.wait_for(main_lock, timeout_duration, [] { return false; }) == false; #if _HAS_CXX20 // because of stop_token case 4: if constexpr (is_same_v) { stop_source source; - return cv.wait_until(lock, source.get_token(), timeout, [] { return false; }) == false; + return cv.wait_until(main_lock, source.get_token(), timeout, [] { return false; }) == false; } else { assert(false); // test not supported for std::condition_variable return false; @@ -165,7 +165,7 @@ namespace { case 5: if constexpr (is_same_v) { stop_source source; - return cv.wait_for(lock, source.get_token(), timeout_duration, [] { return false; }) == false; + return cv.wait_for(main_lock, source.get_token(), timeout_duration, [] { return false; }) == false; } else { assert(false); // test not supported for std::condition_variable return false; From 322e89fc87992442eb5a53acee48671a602997fe Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 24 Nov 2025 14:54:29 -0800 Subject: [PATCH 14/14] Extract steady_clock::now() calls to avoid skew. --- .../GH_000685_condition_variable_any/test.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/std/tests/GH_000685_condition_variable_any/test.cpp b/tests/std/tests/GH_000685_condition_variable_any/test.cpp index a75008d4e8d..1edf99b2c3b 100644 --- a/tests/std/tests/GH_000685_condition_variable_any/test.cpp +++ b/tests/std/tests/GH_000685_condition_variable_any/test.cpp @@ -107,16 +107,15 @@ namespace { atomic_flag waiting_for_other_thread{}; waiting_for_other_thread.test_and_set(); + const auto wait_start = steady_clock::now(); auto timeout_duration = short_timeout; - auto timeout = steady_clock::now() + timeout_duration; + auto timeout = wait_start + timeout_duration; const auto set_timeout = [&](const auto new_timeout) { timeout_duration = new_timeout; timeout = steady_clock::now() + new_timeout; }; - const auto wait_start = steady_clock::now(); - thread other_thread([&] { printf( "thread start after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); @@ -178,18 +177,20 @@ namespace { } }(); + const auto elapsed = steady_clock::now() - wait_start; + if (!cv_wait_timed_out) { if (retries_remaining > 0) { - printf("unexpected wakeup after %lld ms, retry %d...\n", - duration_cast(steady_clock::now() - wait_start).count(), retries_remaining); + printf("unexpected wakeup after %lld ms, retry %d...\n", duration_cast(elapsed).count(), + retries_remaining); test_timeout_immutable(test_number, retries_remaining - 1); // recurse to try the test again } else { puts("Too many unexpected wakeups"); assert(false); } } else { - assert(steady_clock::now() - wait_start < long_timeout / 2); - printf("wait end after %lld ms\n", duration_cast(steady_clock::now() - wait_start).count()); + assert(elapsed < long_timeout / 2); + printf("wait end after %lld ms\n", duration_cast(elapsed).count()); } // Make sure the child thread has indeed finished (so the next join does not block)