On Tue, 28 Jan 2025 at 14:10, Jonathan Wakely wrote:
>
> On Fri, 10 Jan 2025 at 21:29, Jonathan Wakely wrote:
> >
> > + template<typename _Tp,
> > + typename _Pred, typename _ValFn>
> > + void
> > + __atomic_wait_address(const _Tp* __addr,
> > + _Pred&& __pred, _ValFn&& __vfn,
> > + bool __bare_wait = false) noexcept
> > + {
> > + const auto __wait_addr =
> > + reinterpret_cast<const __detail::__platform_wait_t*>(__addr);
> > + __detail::__wait_args __args{ __addr, __bare_wait };
> > + _Tp __val = __vfn();
> > + while (!__pred(__val))
> > + {
>
> There's a serious race condition here, resulting in missed
> notifications and causing __atomic_wait_address to block forever.
>
> The problem case is for proxied waits, where __wait_impl waits for the
> value of _M_ver to be incremented by __notify_impl.
> Let's say thread A checks __pred(__val) here and determines it hasn't
> changed yet, then gets time-sliced and thread B runs. Thread B changes
> __val and calls notify() which increments _M_ver. Then thread A
> continues, calling __wait_impl which loads _M_ver twice, decides it
> hasn't changed, and goes to sleep forever waiting for it to change.
>
> Thread A will never wake up, because thread B already sent its
> notification. Thread A will never notice that __val has changed,
> because it's blocked waiting for _M_ver to change, so never gets to
> check __pred(__val) again.
>
> Before this refactoring, the load of _M_ver happened before checking
> __pred(__val), which avoids the problem (as long as the value of
> _M_ver doesn't wrap and
Oops, I hit Ctrl-Enter instead of Ctrl-Backspace, which is send in gmail.
Loading the initial _M_ver value before checking the predicate works
as long as _M_ver isn't incremented 2^32 times so that it wraps back
to the same value, which would look like not changing. It's not
strictly correct but seems good enough to assume that thread B (and
any other threads) won't do 4 billion notifications in the time it
takes thread A to check the predicate and enter __wait_impl.
But that would be more of a concern for P2643, which wants to allow
arbitrary user-defined predicates to be used with atomic waiting
functions. Those also mean we can't just load the previous value of
_M_ver and check __pred(__val) all while holding a mutex lock, to
properly synchronize with atomic::notify(). Holding a lock around
arbitrary user-defined predicates is a no-no.
