On Fri, 9 Jan 2026 at 08:56, Tomasz Kaminski <[email protected]> wrote:
>
>
>
> 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,

It happens on a non-spurious wake as well, the current code always
calls _M_setup_wait after __wait_impl, even if it woke up because the
value changed and a notify call was done.


> 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 the condition above is true, the we don't need to call
use_proxy_wait, so this should be if-else:

>      if (!use_proxy_wait(*this)) // We can wait on this address directly.
>          return false;

Or maybe you meant to do return true above, after setting _M_old


>     // 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
>>

Reply via email to