[PATCH v5 6/8] libstdc++ atomic_futex: Avoid rounding errors in std::future::wait_* [PR91486]

[Mike Crowe]

On 29/05/20 07:17 +0100, Mike Crowe via Libstdc++ wrote:
> > > diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h
> > > index 5f95ade..aa137a7 100644
> > > --- a/libstdc++-v3/include/bits/atomic_futex.h
> > > +++ b/libstdc++-v3/include/bits/atomic_futex.h
> > > @@ -219,8 +219,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >      _M_load_when_equal_for(unsigned __val, memory_order __mo,
> > > 	  const chrono::duration<_Rep, _Period>& __rtime)
> > >      {
> > > +	using __dur = typename __clock_t::duration;
> > > 	return _M_load_when_equal_until(__val, __mo,
> > > -					__clock_t::now() + __rtime);
> > > +		    __clock_t::now() + chrono::__detail::ceil<__dur>(__rtime));
> > >      }
> > > 
> > >    // Returns false iff a timeout occurred.
> > > @@ -233,7 +234,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > 	do {
> > > 	  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<_Duration>(__delta);

On Friday 11 September 2020 at 18:22:04 +0100, Jonathan Wakely wrote:
> I'm testing the attached patch to fix the C++11 constexpr error, but
> while re-looking at the uses of __detail::ceil I noticed this is using
> _Duration as the target type. Shouldn't that be __clock_t::duration
> instead? Why do we care about the duration of the user's time_point
> here, rather than the preferred duration of the clock we're about to
> wait against?

I think you're right. I've attached a patch to fix it and also add a test
that would have failed at least some of the time if run on a machine with
an uptime greater than 208.5 days with:

 void test_pr91486_wait_until(): Assertion 'float_steady_clock::call_count <= 3' failed.

If we implement the optimisation to not re-check against the custom clock
when the wait is complete if is_steady == true then the test would have
started failing due to the wait not being long enough.

(I used a couple of the GCC farm machines that have high uptimes to test
this.)

Thanks.

Mike.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-libstdc-Use-correct-duration-for-atomic_futex-wait-o.patch
Type: text/x-diff
Size: 5066 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20200919/750a9e19/attachment-0001.bin>