Re: [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock

[Mike Crowe <mac@mcrowe.com>]

On Monday 07 October 2019 at 07:24:58 +0200, François Dumont wrote:
> On 9/28/19 10:44 AM, Mike Crowe wrote:

[snip]

> > diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
> > index e06d286..bb3a41b 100644
> > --- a/libstdc++-v3/include/std/mutex
> > +++ b/libstdc++-v3/include/std/mutex
> > @@ -189,8 +189,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >   	bool
> >   	_M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime)
> >   	{
> > -	  auto __rtime = __atime - _Clock::now();
> > -	  return _M_try_lock_for(__rtime);
> > +          // The user-supplied clock may not tick at the same rate as
> > +          // steady_clock, so we must loop in order to guarantee that
> > +          // the timeout has expired before returning false.
> > +          auto __now = _Clock::now();
> 
> Isn't it possible here that __now == __atime ?
> 
> In this case the loop won't even be entered giving no chance for
> _M_try_lock_for to return true as it used to be done in the previous code.
> 
> So a do/while seems better to me. Or at least a __atime >= __now if the
> Standard says that false shall be returned when __atime < __now even without
> trying to lock.
>
> > +          while (__atime > __now) {
> > +            auto __rtime = __atime - __now;
> > +            if (_M_try_lock_for(__rtime))
> > +              return true;
> > +            __now = _Clock::now();
> > +          }
> > +          return false;
> >   	}
> >       };

You are correct. I'll try to come up with a new test that fails due to this
flaw and check for the same in the rest of the series.

Thanks!

Mike.




> 
>