[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2023-01-18 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

Jonathan Wakely  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|ASSIGNED|RESOLVED
   Target Milestone|--- |11.4

--- Comment #11 from Jonathan Wakely  ---
Fixed for 11.4 and 12.3

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2023-01-18 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

--- Comment #10 from CVS Commits  ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
:

https://gcc.gnu.org/g:ed58809ea1a8ccc1829d830799d34aa51e51d39e

commit r11-10472-ged58809ea1a8ccc1829d830799d34aa51e51d39e
Author: Jonathan Wakely 
Date:   Thu Jul 28 16:15:58 2022 +0100

libstdc++: Unblock atomic wait on non-futex platforms [PR106183]

When using a mutex and condition variable, the notifying thread needs to
increment _M_ver while holding the mutex lock, and the waiting thread
needs to re-check after locking the mutex. This avoids a missed
notification as described in the PR.

By moving the increment of _M_ver to the base _M_notify we can make the
use of the mutex local to the use of the condition variable, and
simplify the code a little. We can use a relaxed store because the mutex
already provides sequential consistency. Also we don't need to check
whether __addr == &_M_ver because we know that's always true for
platforms that use a condition variable, and so we also know that we
always need to use notify_all() not notify_one().

Reviewed-by: Thomas Rodgers 

libstdc++-v3/ChangeLog:

PR libstdc++/106183
* include/bits/atomic_wait.h (__waiter_pool_base::_M_notify):
Move increment of _M_ver here.
[!_GLIBCXX_HAVE_PLATFORM_WAIT]: Lock mutex around increment.
Use relaxed memory order and always notify all waiters.
(__waiter_base::_M_do_wait) [!_GLIBCXX_HAVE_PLATFORM_WAIT]:
Check value again after locking mutex.
(__waiter_base::_M_notify): Remove increment of _M_ver.

(cherry picked from commit af98cb88eb4be6a1668ddf966e975149bf8610b1)

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2023-01-16 Thread i.nixman at autistici dot org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

--- Comment #9 from niXman  ---
looks like it's fixed for x86_64-w64-mingw32.

I used the test from the: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101037

I run it on bash loop for the night and it successfully executed for ~179k
times.

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2023-01-16 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

--- Comment #8 from CVS Commits  ---
The releases/gcc-12 branch has been updated by Jonathan Wakely
:

https://gcc.gnu.org/g:1dfe15e534adba21e680b8128f0631e8054a5e42

commit r12-9047-g1dfe15e534adba21e680b8128f0631e8054a5e42
Author: Jonathan Wakely 
Date:   Thu Jul 28 16:15:58 2022 +0100

libstdc++: Unblock atomic wait on non-futex platforms [PR106183]

When using a mutex and condition variable, the notifying thread needs to
increment _M_ver while holding the mutex lock, and the waiting thread
needs to re-check after locking the mutex. This avoids a missed
notification as described in the PR.

By moving the increment of _M_ver to the base _M_notify we can make the
use of the mutex local to the use of the condition variable, and
simplify the code a little. We can use a relaxed store because the mutex
already provides sequential consistency. Also we don't need to check
whether __addr == &_M_ver because we know that's always true for
platforms that use a condition variable, and so we also know that we
always need to use notify_all() not notify_one().

Reviewed-by: Thomas Rodgers 

libstdc++-v3/ChangeLog:

PR libstdc++/106183
* include/bits/atomic_wait.h (__waiter_pool_base::_M_notify):
Move increment of _M_ver here.
[!_GLIBCXX_HAVE_PLATFORM_WAIT]: Lock mutex around increment.
Use relaxed memory order and always notify all waiters.
(__waiter_base::_M_do_wait) [!_GLIBCXX_HAVE_PLATFORM_WAIT]:
Check value again after locking mutex.
(__waiter_base::_M_notify): Remove increment of _M_ver.

(cherry picked from commit af98cb88eb4be6a1668ddf966e975149bf8610b1)

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2023-01-16 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

Jonathan Wakely  changed:

   What|Removed |Added

 CC||bjornsundin02 at gmail dot com

--- Comment #7 from Jonathan Wakely  ---
*** Bug 101037 has been marked as a duplicate of this bug. ***

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-08-04 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

Jonathan Wakely  changed:

   What|Removed |Added

   Assignee|rodgertq at gcc dot gnu.org|redi at gcc dot gnu.org

--- Comment #6 from Jonathan Wakely  ---
Should be fixed on trunk now.

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-08-04 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

--- Comment #5 from CVS Commits  ---
The master branch has been updated by Jonathan Wakely :

https://gcc.gnu.org/g:af98cb88eb4be6a1668ddf966e975149bf8610b1

commit r13-1957-gaf98cb88eb4be6a1668ddf966e975149bf8610b1
Author: Jonathan Wakely 
Date:   Thu Jul 28 16:15:58 2022 +0100

libstdc++: Unblock atomic wait on non-futex platforms [PR106183]

When using a mutex and condition variable, the notifying thread needs to
increment _M_ver while holding the mutex lock, and the waiting thread
needs to re-check after locking the mutex. This avoids a missed
notification as described in the PR.

By moving the increment of _M_ver to the base _M_notify we can make the
use of the mutex local to the use of the condition variable, and
simplify the code a little. We can use a relaxed store because the mutex
already provides sequential consistency. Also we don't need to check
whether __addr == &_M_ver because we know that's always true for
platforms that use a condition variable, and so we also know that we
always need to use notify_all() not notify_one().

Reviewed-by: Thomas Rodgers 

libstdc++-v3/ChangeLog:

PR libstdc++/106183
* include/bits/atomic_wait.h (__waiter_pool_base::_M_notify):
Move increment of _M_ver here.
[!_GLIBCXX_HAVE_PLATFORM_WAIT]: Lock mutex around increment.
Use relaxed memory order and always notify all waiters.
(__waiter_base::_M_do_wait) [!_GLIBCXX_HAVE_PLATFORM_WAIT]:
Check value again after locking mutex.
(__waiter_base::_M_notify): Remove increment of _M_ver.

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-08-01 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

--- Comment #4 from Jonathan Wakely  ---
Created attachment 53394
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53394=edit
Proposed patch: Unblock atomic wait on non-futex platforms

When using a mutex and condition variable, the notifying thread needs to
increment _M_ver while holding the mutex lock, and the waiting thread
needs to re-check after locking the mutex. This avoids a missed
notification as described in the PR.

By moving the increment of _M_ver to the base _M_notify we can make the
use of the mutex local to the use of the condition variable, and
simplify the code a little. We can use a relaxed store because the mutex
already provides sequential consistency. Also we don't need to check
whether __addr == &_M_ver because we know that's always true for
platforms that use a condition variable, and so we also know that we
always need to use notify_all() not notify_one().

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-07-25 Thread anthony.ajw at gmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

Anthony Williams  changed:

   What|Removed |Added

 CC||anthony.ajw at gmail dot com

--- Comment #3 from Anthony Williams  ---
This is one of the common mistakes I mention when teaching people about
condition variables. Just because the data being waited for is atomic, doesn't
guarantee that the condition variable state is updated: you need the mutex to
synchronize that.

In current libstdc++ trunk libstdc++-v3/include/bits/atomic_wait.h insert a
line in _M_notify at line 235:

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/atomic_wait.h;h=125b1cad88682384737c048ac236af9c4deab957;hb=refs/heads/trunk

  void
  _M_notify(const __platform_wait_t* __addr, bool __all, bool __bare)
noexcept
  {
if (!(__bare || _M_waiting()))
  return;

#ifdef _GLIBCXX_HAVE_PLATFORM_WAIT
__platform_notify(__addr, __all);
#else
/// INSERT HERE
{ std::lock_guard __lock(_M_mtx); }
/// END INSERT
if (__all)
  _M_cv.notify_all();
else
  _M_cv.notify_one();
#endif
  }

The lock/unlock here ensures that the notify is correctly synchronized with the
wait.

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-07-15 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

--- Comment #2 from Jonathan Wakely  ---
(In reply to Ted_lion from comment #1)
> The subprocedure of comparison
> with old value and waiting for the notification should not be interrupted by
> an operation on same atomic object.

I'm not sure what you mean here, could you clarify please?

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-07-15 Thread tedlion_tang at foxmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

Ted_lion  changed:

   What|Removed |Added

 CC||tedlion_tang at foxmail dot com

--- Comment #1 from Ted_lion  ---
I found my program blocked and it turned out to be the same problem of
atomic::wait. According to the C++20 Standard, the function atomic::wait should
perform an atomic waiting operations. The subprocedure of comparison with old
value and waiting for the notification should not be interrupted by an
operation on same atomic object.

[Bug libstdc++/106183] std::atomic::wait might fail to be unblocked by notify_one/all on platforms without platform_wait()

2022-07-05 Thread rodgertq at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106183

Thomas Rodgers  changed:

   What|Removed |Added

 Ever confirmed|0   |1
   Last reconfirmed||2022-07-06
 Status|UNCONFIRMED |ASSIGNED