diff options
author | Mike Crowe <mac@mcrowe.com> | 2020-09-11 14:25:00 +0100 |
---|---|---|
committer | Jonathan Wakely <jwakely@redhat.com> | 2020-09-11 14:28:50 +0100 |
commit | e05ff30078e80869f2bf3af6dbdbea134c252158 (patch) | |
tree | 6c76570dc68fb584ed2a54c7d05007fda53b4347 | |
parent | f9ddb696a289cc48d24d3d23c0b324cb88de9573 (diff) | |
download | gcc-e05ff30078e80869f2bf3af6dbdbea134c252158.zip gcc-e05ff30078e80869f2bf3af6dbdbea134c252158.tar.gz gcc-e05ff30078e80869f2bf3af6dbdbea134c252158.tar.bz2 |
libstdc++: Avoid rounding errors on custom clocks in condition_variable
The fix for PR68519 in 83fd5e73b3c16296e0d7ba54f6c547e01c7eae7b only
applied to condition_variable::wait_for. This problem can also apply to
condition_variable::wait_until but only if the custom clock is using a
more recent epoch so that a small enough delta can be calculated. let's
use the newly-added chrono::__detail::ceil to fix this and also make use
of that function to simplify the previous wait_for fixes.
Also, simplify the existing test case for PR68519 a little and make its
variables local so we can add a new test case for the above problem.
Unfortunately, the test would have only started failing if sufficient
time has passed since the chrono::steady_clock epoch had passed anyway,
but it's better than nothing.
libstdc++-v3/ChangeLog:
* include/std/condition_variable (condition_variable::wait_until):
Convert delta to steady_clock duration before adding to current
steady_clock time to avoid rounding errors described in PR68519.
(condition_variable::wait_for): Simplify calculation of absolute
time by using chrono::__detail::ceil in both overloads.
* testsuite/30_threads/condition_variable/members/68519.cc:
(test_wait_for): Renamed from test01. Replace unassigned val
variable with constant false. Reduce scope of mx and cv
variables to just test_wait_for function.
(test_wait_until): Add new test case.
-rw-r--r-- | libstdc++-v3/include/std/condition_variable | 18 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc | 61 |
2 files changed, 63 insertions, 16 deletions
diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable index a08cfc6..1abec54 100644 --- a/libstdc++-v3/include/std/condition_variable +++ b/libstdc++-v3/include/std/condition_variable @@ -133,10 +133,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus > 201703L static_assert(chrono::is_clock_v<_Clock>); #endif + using __s_dur = typename __clock_t::duration; const typename _Clock::time_point __c_entry = _Clock::now(); const __clock_t::time_point __s_entry = __clock_t::now(); const auto __delta = __atime - __c_entry; - const auto __s_atime = __s_entry + __delta; + const auto __s_atime = __s_entry + + chrono::__detail::ceil<__s_dur>(__delta); if (__wait_until_impl(__lock, __s_atime) == cv_status::no_timeout) return cv_status::no_timeout; @@ -166,10 +168,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION const chrono::duration<_Rep, _Period>& __rtime) { using __dur = typename steady_clock::duration; - auto __reltime = chrono::duration_cast<__dur>(__rtime); - if (__reltime < __rtime) - ++__reltime; - return wait_until(__lock, steady_clock::now() + __reltime); + return wait_until(__lock, + steady_clock::now() + + chrono::__detail::ceil<__dur>(__rtime)); } template<typename _Rep, typename _Period, typename _Predicate> @@ -179,10 +180,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Predicate __p) { using __dur = typename steady_clock::duration; - auto __reltime = chrono::duration_cast<__dur>(__rtime); - if (__reltime < __rtime) - ++__reltime; - return wait_until(__lock, steady_clock::now() + __reltime, + return wait_until(__lock, + steady_clock::now() + + chrono::__detail::ceil<__dur>(__rtime), std::move(__p)); } diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc index 2a6ecb9..c86ca2c 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc @@ -25,25 +25,72 @@ // PR libstdc++/68519 -bool val = false; -std::mutex mx; -std::condition_variable cv; - void -test01() +test_wait_for() { + std::mutex mx; + std::condition_variable cv; + for (int i = 0; i < 3; ++i) { std::unique_lock<std::mutex> l(mx); auto start = std::chrono::system_clock::now(); - cv.wait_for(l, std::chrono::duration<float>(1), [] { return val; }); + cv.wait_for(l, std::chrono::duration<float>(1), [] { return false; }); auto t = std::chrono::system_clock::now(); VERIFY( (t - start) >= std::chrono::seconds(1) ); } } +// In order to ensure that the delta calculated in the arbitrary clock overload +// of condition_variable::wait_until fits accurately in a float, but the result +// of adding it to steady_clock with a float duration does not, this clock +// needs to use a more recent epoch. +struct recent_epoch_float_clock +{ + using rep = std::chrono::duration<float>::rep; + using period = std::chrono::duration<float>::period; + using time_point = std::chrono::time_point<recent_epoch_float_clock, + std::chrono::duration<float>>; + static constexpr bool is_steady = true; + + static const std::chrono::steady_clock::time_point epoch; + + static time_point now() + { + const auto steady = std::chrono::steady_clock::now(); + return time_point{steady - epoch}; + } +}; + +const std::chrono::steady_clock::time_point recent_epoch_float_clock::epoch = + std::chrono::steady_clock::now(); + +void +test_wait_until() +{ + using clock = recent_epoch_float_clock; + + std::mutex mx; + std::condition_variable cv; + + for (int i = 0; i < 3; ++i) + { + std::unique_lock<std::mutex> l(mx); + const auto start = clock::now(); + const auto wait_time = start + std::chrono::duration<float>{1.0}; + + // In theory we could get a spurious wakeup, but in practice we won't. + const auto result = cv.wait_until(l, wait_time); + + VERIFY( result == std::cv_status::timeout ); + const auto elapsed = clock::now() - start; + VERIFY( elapsed >= std::chrono::seconds(1) ); + } +} + int main() { - test01(); + test_wait_for(); + test_wait_until(); } |