[PATCH v3] libstdc++: Make atomic<shared_ptr<T>>::wait sensitive to stored pointer only changes [PR118757]
Jonathan Wakely
jwakely@redhat.com
Tue Oct 14 12:34:14 GMT 2025
On Tue, 14 Oct 2025 at 13:57 +0200, Tomasz Kamiński wrote:
>Previously, atomic<shared_ptr<T>>::wait (and the weak_ptr version) was
>equivalent to waiting directly on _M_val, which corresponds to the pointer
>to the control block (_M_pi). Consequently, wakeups were not triggered if
>the stored pointer value was changed to a pointer that uses the same control
>block but stores pointer to a different object. Such a pointer can be
>constructed using an aliasing constructor.
>
>To address this, wait now uses a generic proxy wait
>std::__atomic_wait_address function, which supports waiting until any
>predicate is satisfied. The provided predicate now compares both the control
>block (_M_pi) and the stored pointer (_M_ptr). Comparing the latter requires
>locking the pointer.
>
>Since this function operates on raw pointers, the type of _M_val was changed
>from __atomic_base<uintptr_t> to uintptr_t. Invocations of the corresponding
>member functions are now replaced with direct use of __atomic builtins.
>
> PR libstdc++/118757
>
>libstdc++-v3/ChangeLog:
>
> * include/bits/shared_ptr_atomic.h (_Atomic_count::_M_wait_unlock):
> Add parameter capturing reference to _M_ptr. Reimplement in terms
> of __atomic_wait_address.
> (_Atomic_count::_AtomicRef): Define.
> (_Atomic_count::~_Atomic_count, _Atomic_count::lock)
> (_Atomic_count::unlock, _Atomic_count::_M_swap_unlock)
> (_Atomic_count::notify_one, _Atomic_count::notify_all):
> Use _AtomicRef for atomic operations on _M_val.
> (_Sp_atomic::element_type): Define.
> (_Sp_atomic::_M_val): Change type to uintptr_t, add appropariate
> alignas.
> (_Sp_atomic::wait): Pass _M_ptr to _M_wait_unlock.
> * python/libstdcxx/v6/printers.py:
> * testsuite/20_util/shared_ptr/atomic/pr118757.cc: New test.
> * testsuite/20_util/weak_ptr/pr118757.cc: New test.
>
>Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>
>---
>changes in v3:
> * break the lines to fit 80 collumns in commit description
> * fix typos in commit message.
> * make sure that _M_val is sufficiently aligned
> (atomic_ref<uintptr_t>::required_aligment)
> * using __atomic_ref<uintptr_t> for atomic operations on _M_val.
> * remove copyright notice and fix signaller in test.
>
>Testing on x86_64-linux. OK for trunk when test passes?
OK, thanks
> libstdc++-v3/include/bits/shared_ptr_atomic.h | 57 ++++++++++++++-----
> libstdc++-v3/python/libstdcxx/v6/printers.py | 6 +-
> .../20_util/shared_ptr/atomic/pr118757.cc | 29 ++++++++++
> .../testsuite/20_util/weak_ptr/pr118757.cc | 30 ++++++++++
> 4 files changed, 107 insertions(+), 15 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc
> create mode 100644 libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc
>
>diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h b/libstdc++-v3/include/bits/shared_ptr_atomic.h
>index cc7841a8775..cbc4bf621f4 100644
>--- a/libstdc++-v3/include/bits/shared_ptr_atomic.h
>+++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h
>@@ -392,6 +392,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> class _Sp_atomic
> {
> using value_type = _Tp;
>+ using element_type = typename _Tp::element_type;
>
> friend struct atomic<_Tp>;
>
>@@ -420,7 +421,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> ~_Atomic_count()
> {
>- auto __val = _M_val.load(memory_order_relaxed);
>+ auto __val = _AtomicRef(_M_val).load(memory_order_relaxed);
> _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val);
> __glibcxx_assert(!(__val & _S_lock_bit));
> if (auto __pi = reinterpret_cast<pointer>(__val))
>@@ -442,18 +443,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> // To acquire the lock we flip the LSB from 0 to 1.
>
>- auto __current = _M_val.load(memory_order_relaxed);
>+ _AtomicRef __aref(_M_val);
>+ auto __current = __aref.load(memory_order_relaxed);
> while (__current & _S_lock_bit)
> {
> #if __glibcxx_atomic_wait
> __detail::__thread_relax();
> #endif
>- __current = _M_val.load(memory_order_relaxed);
>+ __current = __aref.load(memory_order_relaxed);
> }
>
> _GLIBCXX_TSAN_MUTEX_TRY_LOCK(&_M_val);
>
>- while (!_M_val.compare_exchange_strong(__current,
>+ while (!__aref.compare_exchange_strong(__current,
> __current | _S_lock_bit,
> __o,
> memory_order_relaxed))
>@@ -474,7 +476,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> unlock(memory_order __o) const noexcept
> {
> _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val);
>- _M_val.fetch_sub(1, __o);
>+ _AtomicRef(_M_val).fetch_sub(1, __o);
> _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val);
> }
>
>@@ -487,7 +489,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __o = memory_order_release;
> auto __x = reinterpret_cast<uintptr_t>(__c._M_pi);
> _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val);
>- __x = _M_val.exchange(__x, __o);
>+ __x = _AtomicRef(_M_val).exchange(__x, __o);
> _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val);
> __c._M_pi = reinterpret_cast<pointer>(__x & ~_S_lock_bit);
> }
>@@ -495,19 +497,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> #if __glibcxx_atomic_wait
> // Precondition: caller holds lock!
> void
>- _M_wait_unlock(memory_order __o) const noexcept
>+ _M_wait_unlock(const element_type* const& __ptr, memory_order __o) const noexcept
> {
>+ auto __old_ptr = __ptr;
> _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val);
>- auto __v = _M_val.fetch_sub(1, memory_order_relaxed);
>+ uintptr_t __old_pi
>+ = _AtomicRef(_M_val).fetch_sub(1, memory_order_relaxed) - 1u;
> _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val);
>- _M_val.wait(__v & ~_S_lock_bit, __o);
>+
>+ // Ensure that the correct value of _M_ptr is visible after locking,
>+ // by upgrading relaxed or consume to acquire.
>+ auto __lo = __o;
>+ if (__o != memory_order_seq_cst)
>+ __lo = memory_order_acquire;
>+
>+ std::__atomic_wait_address(
>+ &_M_val,
>+ [=, &__ptr, this](uintptr_t __new_pi)
>+ {
>+ if (__old_pi != (__new_pi & ~_S_lock_bit))
>+ // control block changed, we can wake up
>+ return true;
>+
>+ // control block is same, we need to check if ptr changed,
>+ // the lock needs to be taken first, the value of pi may have
>+ // also been updated in meantime, so reload it
>+ __new_pi = reinterpret_cast<uintptr_t>(this->lock(__lo));
>+ auto __new_ptr = __ptr;
>+ this->unlock(memory_order_relaxed);
>+ // wake up if either of the values changed
>+ return __new_pi != __old_pi || __new_ptr != __old_ptr;
>+ },
>+ [__o, this] { return _AtomicRef(_M_val).load(__o); });
> }
>
> void
> notify_one() noexcept
> {
> _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(&_M_val);
>- _M_val.notify_one();
>+ _AtomicRef(_M_val).notify_one();
> _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(&_M_val);
> }
>
>@@ -515,17 +543,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> notify_all() noexcept
> {
> _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(&_M_val);
>- _M_val.notify_all();
>+ _AtomicRef(_M_val).notify_all();
> _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(&_M_val);
> }
> #endif
>
> private:
>- mutable __atomic_base<uintptr_t> _M_val{0};
>+ using _AtomicRef = __atomic_ref<uintptr_t>;
>+ alignas(_AtomicRef::required_alignment) mutable uintptr_t _M_val{0};
> static constexpr uintptr_t _S_lock_bit{1};
> };
>
>- typename _Tp::element_type* _M_ptr = nullptr;
>+ element_type* _M_ptr = nullptr;
> _Atomic_count _M_refcount;
>
> static typename _Atomic_count::pointer
>@@ -608,7 +637,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> auto __pi = _M_refcount.lock(memory_order_acquire);
> if (_M_ptr == __old._M_ptr && __pi == __old._M_refcount._M_pi)
>- _M_refcount._M_wait_unlock(__o);
>+ _M_refcount._M_wait_unlock(_M_ptr, __o);
> else
> _M_refcount.unlock(memory_order_relaxed);
> }
>diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
>index e5336b7fa89..1822d425f88 100644
>--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
>+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
>@@ -287,7 +287,11 @@ class SharedPointerPrinter(printer_base):
> def _get_refcounts(self):
> if self._typename == 'std::atomic':
> # A tagged pointer is stored as uintptr_t.
>- ptr_val = self._val['_M_refcount']['_M_val']['_M_i']
>+ val = self._val['_M_refcount']['_M_val']
>+ if val.type.is_scalar: # GCC 16 stores uintptr_t
>+ ptr_val = val
>+ else: # GCC 12-15 stores std::atomic<uintptr_t>
>+ ptr_val = val['_M_i']
> ptr_val = ptr_val - (ptr_val % 2) # clear lock bit
> ptr_type = find_type(self._val['_M_refcount'].type, 'pointer')
> return ptr_val.cast(ptr_type)
>diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc
>new file mode 100644
>index 00000000000..d54abd8a039
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/shared_ptr/atomic/pr118757.cc
>@@ -0,0 +1,29 @@
>+// { dg-do run { target c++20 } }
>+// { dg-require-gthreads "" }
>+// { dg-require-effective-target hosted }
>+
>+#include <memory>
>+#include <chrono>
>+#include <thread>
>+#include <barrier>
>+
>+std::shared_ptr<int> q = std::make_shared<int>(42);
>+std::atomic<std::shared_ptr<int>> p = q;
>+
>+std::barrier bar(2);
>+
>+void signaller()
>+{
>+ std::this_thread::sleep_for(std::chrono::seconds(1));
>+ p.store(std::shared_ptr<int>(q, nullptr));
>+ p.notify_one();
>+ bar.arrive_and_wait();
>+}
>+
>+int main(int, char**)
>+{
>+ std::thread thr(signaller);
>+ p.wait(q);
>+ bar.arrive_and_wait();
>+ thr.join();
>+}
>diff --git a/libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc b/libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc
>new file mode 100644
>index 00000000000..f048f13aec2
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/weak_ptr/pr118757.cc
>@@ -0,0 +1,30 @@
>+// { dg-do run { target c++20 } }
>+// { dg-require-gthreads "" }
>+// { dg-require-effective-target hosted }
>+
>+#include <memory>
>+#include <chrono>
>+#include <thread>
>+#include <barrier>
>+
>+std::shared_ptr<int> s = std::make_shared<int>(42);
>+std::weak_ptr<int> q = s;
>+std::atomic<std::weak_ptr<int>> p = q;
>+
>+std::barrier bar(2);
>+
>+void signaller()
>+{
>+ std::this_thread::sleep_for(std::chrono::seconds(1));
>+ p.store(std::shared_ptr<int>(s, nullptr));
>+ p.notify_one();
>+ bar.arrive_and_wait();
>+}
>+
>+int main(int, char**)
>+{
>+ std::thread thr(signaller);
>+ p.wait(q);
>+ bar.arrive_and_wait();
>+ thr.join();
>+}
>--
>2.51.0
>
>
More information about the Libstdc++
mailing list