public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r11-3158] libstdc++: Avoid rounding errors on custom clocks in condition_variable
@ 2020-09-11 13:29 Jonathan Wakely
0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2020-09-11 13:29 UTC (permalink / raw)
To: gcc-cvs, libstdc++-cvs
https://gcc.gnu.org/g:e05ff30078e80869f2bf3af6dbdbea134c252158
commit r11-3158-ge05ff30078e80869f2bf3af6dbdbea134c252158
Author: Mike Crowe <mac@mcrowe.com>
Date: Fri Sep 11 14:25:00 2020 +0100
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.
Diff:
---
libstdc++-v3/include/std/condition_variable | 18 +++----
.../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 a08cfc62705..1abec546320 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 2a6ecb95fd1..c86ca2ccdbf 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();
}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2020-09-11 13:29 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 13:29 [gcc r11-3158] libstdc++: Avoid rounding errors on custom clocks in condition_variable Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).