On Thu, Jan 8, 2026 at 10:50 PM Jonathan Wakely <[email protected]> wrote:
> A failed assertion was observed with std::atomic<bool>::wait when the
> loop in __atomic_wait_address is entered and calls _M_setup_wait a
> second time. When the first call to _M_setup_wait makes a call to
> _M_setup_proxy_wait it decides that a proxy wait is needed for bool, and
> updates the _M_obj and _M_obj_size members to refer to the futex in the
> proxy state, instead of referring to the bool object. The next time
> _M_setup_wait is called it calls _M_setup_proxy_wait again but now it
> sees _M_obj_size == sizeof(futex) and so this time decides a proxy wait
> is *not* needed, and then fails the __glibcxx_assert(_M_obj == addr)
> check.
>
> The problem is that _M_setup_wait is calling _M_setup_proxy_wait twice,
> when it should avoid the second call because the decision to use a proxy
> wait has already been made. The caller should avoid making the second
> call by checking whether _M_obj != addr, because that implies that a
> decision to make a proxy wait has already happened.
>
I do not think this is correct behavior, the _M_setup_wait have two
responsibilities now:
* deciding it the proxy wait should be used
* updating the _M_old value with the most recent _M_ver versions, I am
referring to this line
// Read the value of the _M_ver counter.
_M_old = __atomic_load_n(&state->_M_ver, __ATOMIC_ACQUIRE);
To hit this assertion, this means that there was contention on the proxy
wait slot
or had a spurious wake, and in this case _M_ver was changed, and we should
still update the _M_old to the latest value.
I think the proper fix matching the approach would be to have:
bool
__wait_args::_M_setup_proxy_wait(const void* addr)
{
if (_M_obj != addr) {
// We already decided to use proxy wait, read the value of the _M_ver
counter.
_M_old = __atomic_load_n(&state->_M_ver, __ATOMIC_ACQUIRE);
}
if (!use_proxy_wait(*this)) // We can wait on this address directly.
return false;
// This will be a proxy wait, so get a waitable state.
auto state = set_wait_state(addr, *this);
// The address we will wait on is the version count of the waitable
state:
_M_obj = &state->_M_ver;
// __wait_impl and __wait_until_impl need to know this size:
_M_obj_size = sizeof(state->_M_ver);
}
}
// Read the value of the _M_ver counter.
_M_old = __atomic_load_n(&state->_M_ver, __ATOMIC_ACQUIRE);
return true;
}
// See below for alternative
>
> This change fixes the bug, by preventing a second call to
> _M_setup_proxy_wait after we've changed _M_obj and _M_obj_size. But it
> doesn't prevent a second call in the case where _M_setup_proxy_wait
> returned false because we made a runtime decision to do a non-proxy
> wait. That is suboptimal, because it means that code which can wait on
> the atomic variable directly makes redundant calls into the library just
> to be told the same answer every time. That's not a problem for now
> because firstly, it's only a performance degradation not a correctness
> bug, and secondly because we currently have no targets that ever make a
> runtime decision to do a non-proxy wait. Linux makes a compile-time
> decision to do non-proxy waits for futex-sized objects and does proxy
> waits for everything else, and all non-Linux targets always do proxy
> waits. So solving the performance problem for non-proxy waits can happen
> later. We might want to set a bit in __wait_args::_M_flags to say "I
> already told you this isn't a proxy wait, stop asking me".
>
I think the best option would be to separate _M_setup_wait (initial setups)
and later updates into separate functions. Like _M_setup_wait and
_M_update_wait.
I will post a patch implementing those suggestion.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/atomic_wait.h (__wait_args::_M_setup_wait): Do
> not call _M_setup_proxy_wait again if a proxy wait was already
> set up.
> ---
>
> Tested x86_64-linux and x86_64-freebsd14 (just the parts that use atomic
> waits).
>
> libstdc++-v3/include/bits/atomic_wait.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/bits/atomic_wait.h
> b/libstdc++-v3/include/bits/atomic_wait.h
> index b6240a95370f..a677dbf0a040 100644
> --- a/libstdc++-v3/include/bits/atomic_wait.h
> +++ b/libstdc++-v3/include/bits/atomic_wait.h
> @@ -230,7 +230,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> if constexpr (!__platform_wait_uses_type<_Tp>)
> - if (_M_setup_proxy_wait(__addr))
> + if (_M_obj != __addr || _M_setup_proxy_wait(__addr))
>
Or to reload _M_wait here.
> {
> // We will use a proxy wait for this object.
> // The library has set _M_obj and _M_obj_size and _M_old.
> --
> 2.52.0
>
>
