libstdc++: Fix conformance issues in <stop_token> (PR92895)

Message ID 20200123222028.GA2969@redhat.com
State New
Headers show
Series
  • libstdc++: Fix conformance issues in <stop_token> (PR92895)
Related show

Commit Message

Jonathan Wakely Jan. 23, 2020, 10:20 p.m.
Fix synchronization issues in <stop_token>. Replace shared_ptr with
_Stop_state_ref and a reference count embedded in the shared state.
Replace std::mutex with spinlock using one bit of a std::atomic<> that
also tracks whether a stop request has been made and how many
stop_source objects share ownership of the state.

The synchronization with callbacks being destroyed is based on the
implementation by Lewis Baker and Nico Josuttis. It allows the
callback being destroyed to detect whether it's currently running, and
if so whether on the current thread or a different one.

Tom, please take a look and give a review. As discussed, the
binary_semaphore is temporary, until we have the real thing.

Comments

Thomas Rodgers Jan. 29, 2020, 5:57 p.m. | #1
Looks good to me, ok for trunk.

Thanks.

Jonathan Wakely writes:

> Fix synchronization issues in <stop_token>. Replace shared_ptr with

> _Stop_state_ref and a reference count embedded in the shared state.

> Replace std::mutex with spinlock using one bit of a std::atomic<> that

> also tracks whether a stop request has been made and how many

> stop_source objects share ownership of the state.

>

> The synchronization with callbacks being destroyed is based on the

> implementation by Lewis Baker and Nico Josuttis. It allows the

> callback being destroyed to detect whether it's currently running, and

> if so whether on the current thread or a different one.

>

> Tom, please take a look and give a review. As discussed, the

> binary_semaphore is temporary, until we have the real thing.
Jonathan Wakely Jan. 29, 2020, 6:32 p.m. | #2
On 29/01/20 09:57 -0800, Thomas Rodgers wrote:
>Looks good to me, ok for trunk.


Committed.

Patch

commit 011476e4282eca13d19ca18f21bc010f40134ac5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 23 16:46:17 2020 +0000

    libstdc++: Fix conformance issues in <stop_token> (PR92895)
    
    Fix synchronization issues in <stop_token>. Replace shared_ptr with
    _Stop_state_ref and a reference count embedded in the shared state.
    Replace std::mutex with spinlock using one bit of a std::atomic<> that
    also tracks whether a stop request has been made and how many
    stop_source objects share ownership of the state.
    
            PR libstdc++/92895
            * include/std/stop_token (stop_token::stop_possible()): Call new
            _M_stop_possible() function.
            (stop_token::stop_requested()): Do not use stop_possible().
            (stop_token::binary_semaphore): New class, as temporary stand-in for
            std::binary_semaphore.
            (stop_token::_Stop_cb::_M_callback): Add noexcept to type.
            (stop_token::_Stop_cb::_M_destroyed, stop_token::_Stop_cb::_M_done):
            New data members for symchronization with stop_callback destruction.
            (stop_token::_Stop_cb::_Stop_cb): Make non-template.
            (stop_token::_Stop_cb::_M_linked, stop_token::_Stop_cb::_S_execute):
            Remove.
            (stop_token::_Stop_cb::_M_run): New member function.
            (stop_token::_Stop_state::_M_stopped, stop_token::_Stop_state::_M_mtx):
            Remove.
            (stop_token::_Stop_state::_M_owners): New data member to track
            reference count for ownership.
            (stop_token::_Stop_state::_M_value): New data member combining a
            spinlock, the stop requested flag, and the reference count for
            associated stop_source objects.
            (stop_token::_Stop_state::_M_requester): New data member for
            synchronization with stop_callback destruction.
            (stop_token::_Stop_state::_M_stop_possible()): New member function.
            (stop_token::_Stop_state::_M_stop_requested()): Inspect relevant bit
            of _M_value.
            (stop_token::_Stop_state::_M_add_owner)
            (stop_token::_Stop_state::_M_release_ownership)
            (stop_token::_Stop_state::_M_add_ssrc)
            (stop_token::_Stop_state::_M_sub_ssrc): New member functions for
            updating reference counts.
            (stop_token::_Stop_state::_M_lock, stop_token::_Stop_state::_M_unlock)
            (stop_token::_Stop_state::_M_lock, stop_token::_Stop_state::_M_unlock)
            (stop_token::_Stop_state::_M_try_lock)
            (stop_token::_Stop_state::_M_try_lock_and_stop)
            (stop_token::_Stop_state::_M_do_try_lock): New member functions for
            managing spinlock.
            (stop_token::_Stop_state::_M_request_stop): Use atomic operations to
            read and update state. Release lock while running callbacks. Use new
            data members to synchronize with callback destruction.
            (stop_token::_Stop_state::_M_remove_callback): Likewise.
            (stop_token::_Stop_state::_M_register_callback): Use atomic operations
            to read and update state.
            (stop_token::_Stop_state_ref): Handle type to manage _Stop_state,
            replacing shared_ptr.
            (stop_source::stop_source(const stop_source&)): Update reference count.
            (stop_source::operator=(const stop_source&)): Likewise.
            (stop_source::~stop_source()): Likewise.
            (stop_source::stop_source(stop_source&&)): Define as defaulted.
            (stop_source::operator=(stop_source&&)): Establish postcondition on
            parameter.
            (stop_callback): Enforce preconditions on template parameter. Replace
            base class with data member of new _Cb_impl type.
            (stop_callback::stop_callback(const stop_token&, Cb&&))
            (stop_callback::stop_callback(stop_token&&, Cb&&)): Fix TOCTTOU race.
            (stop_callback::_Cb_impl): New type wrapping _Callback member and
            defining the _S_execute member function.
            * testsuite/30_threads/stop_token/stop_callback/deadlock-mt.cc: New
            test.
            * testsuite/30_threads/stop_token/stop_callback/deadlock.cc: New test.
            * testsuite/30_threads/stop_token/stop_callback/destroy.cc: New test.
            * testsuite/30_threads/stop_token/stop_callback/destructible_neg.cc:
            New test.
            * testsuite/30_threads/stop_token/stop_callback/invocable_neg.cc: New
            test.
            * testsuite/30_threads/stop_token/stop_callback/invoke.cc: New test.
            * testsuite/30_threads/stop_token/stop_source/assign.cc: New test.
            * testsuite/30_threads/stop_token/stop_token/stop_possible.cc: New
            test.

diff --git a/libstdc++-v3/include/std/stop_token b/libstdc++-v3/include/std/stop_token
index e23d139e66c..af4c49aefeb 100644
--- a/libstdc++-v3/include/std/stop_token
+++ b/libstdc++-v3/include/std/stop_token
@@ -32,13 +32,13 @@ 
 #if __cplusplus > 201703L
 
 #include <atomic>
-#include <bits/std_mutex.h>
-#include <ext/concurrence.h>
-#include <bits/unique_ptr.h>
-#include <bits/shared_ptr.h>
 
 #ifdef _GLIBCXX_HAS_GTHREADS
 # define __cpp_lib_jthread 201907L
+# include <bits/gthr.h>
+# if __has_include(<semaphore>)
+#  include <semaphore>
+# endif
 #endif
 
 namespace std _GLIBCXX_VISIBILITY(default)
@@ -49,35 +49,37 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   struct nostopstate_t { explicit nostopstate_t() = default; };
   inline constexpr nostopstate_t nostopstate{};
 
+  class stop_source;
+
   /// Allow testing whether a stop request has been made on a `stop_source`.
   class stop_token
   {
   public:
     stop_token() noexcept = default;
 
-    stop_token(const stop_token& __other) noexcept = default;
-    stop_token(stop_token&& __other) noexcept = default;
+    stop_token(const stop_token&) noexcept = default;
+    stop_token(stop_token&&) noexcept = default;
 
     ~stop_token() = default;
 
     stop_token&
-    operator=(const stop_token& __rhs) noexcept = default;
+    operator=(const stop_token&) noexcept = default;
 
     stop_token&
-    operator=(stop_token&& __rhs) noexcept = default;
+    operator=(stop_token&&) noexcept = default;
 
     [[nodiscard]]
     bool
     stop_possible() const noexcept
     {
-      return static_cast<bool>(_M_state);
+      return static_cast<bool>(_M_state) && _M_state->_M_stop_possible();
     }
 
     [[nodiscard]]
     bool
     stop_requested() const noexcept
     {
-      return stop_possible() && _M_state->_M_stop_requested();
+      return static_cast<bool>(_M_state) && _M_state->_M_stop_requested();
     }
 
     void
@@ -98,76 +100,209 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     template<typename _Callback>
       friend class stop_callback;
 
+    static void
+    _S_yield() noexcept
+    {
+#if defined __i386__ || defined __x86_64__
+	      __builtin_ia32_pause();
+#elif defined _GLIBCXX_USE_SCHED_YIELD
+	      __gthread_yield();
+#endif
+    }
+
+#ifndef __cpp_lib_semaphore
+    // TODO: replace this with a real implementation of std::binary_semaphore
+    struct binary_semaphore
+    {
+      explicit binary_semaphore(int __d) : _M_counter(__d > 0) { }
+
+      void release() { _M_counter.fetch_add(1, memory_order::release); }
+
+      void acquire()
+      {
+	int __old = 1;
+	while (!_M_counter.compare_exchange_weak(__old, 0,
+						 memory_order::acquire,
+						 memory_order::relaxed))
+	  {
+	    __old = 1;
+	    _S_yield();
+	  }
+      }
+
+      atomic<int> _M_counter;
+    };
+#endif
+
     struct _Stop_cb
     {
-      void(*_M_callback)(_Stop_cb*);
+      using __cb_type = void(_Stop_cb*) noexcept;
+      __cb_type* _M_callback;
       _Stop_cb* _M_prev = nullptr;
       _Stop_cb* _M_next = nullptr;
+      bool* _M_destroyed = nullptr;
+      binary_semaphore _M_done{0};
 
-      template<typename _Cb>
-	_Stop_cb(_Cb&& __cb)
-	: _M_callback(std::forward<_Cb>(__cb))
-	{ }
+      [[__gnu__::__nonnull__]]
+      explicit
+      _Stop_cb(__cb_type* __cb)
+      : _M_callback(__cb)
+      { }
 
-      bool
-      _M_linked() const noexcept
-      {
-        return (_M_prev != nullptr)
-          || (_M_next != nullptr);
-      }
-
-      static void
-      _S_execute(_Stop_cb* __cb) noexcept
-      {
-        __cb->_M_callback(__cb);
-        __cb->_M_prev = __cb->_M_next = nullptr;
-      }
+      void _M_run() noexcept { _M_callback(this); }
     };
 
     struct _Stop_state_t
     {
-      std::atomic<bool> _M_stopped{false};
+      using value_type = uint32_t;
+      static constexpr value_type _S_stop_requested_bit = 1;
+      static constexpr value_type _S_locked_bit = 2;
+      static constexpr value_type _S_ssrc_counter_inc = 4;
+
+      std::atomic<value_type> _M_owners{1};
+      std::atomic<value_type> _M_value{_S_ssrc_counter_inc};
       _Stop_cb* _M_head = nullptr;
-#ifdef _GLIBCXX_HAS_GTHREADS
-      std::mutex _M_mtx;
+#if _GLIBCXX_HAS_GTHREADS
+      __gthread_t _M_requester;
 #endif
 
       _Stop_state_t() = default;
 
+      bool
+      _M_stop_possible() noexcept
+      {
+	// true if a stop request has already been made or there are still
+	// stop_source objects that would allow one to be made.
+	return _M_value.load(memory_order::acquire) & ~_S_locked_bit;
+      }
+
       bool
       _M_stop_requested() noexcept
       {
-        return _M_stopped;
+        return _M_value.load(memory_order::acquire) & _S_stop_requested_bit;
+      }
+
+      void
+      _M_add_owner() noexcept
+      {
+	_M_owners.fetch_add(1, memory_order::relaxed);
+      }
+
+      void
+      _M_release_ownership() noexcept
+      {
+	if (_M_owners.fetch_sub(1, memory_order::release) == 1)
+	  delete this;
+      }
+
+      void
+      _M_add_ssrc() noexcept
+      {
+	_M_value.fetch_add(_S_ssrc_counter_inc, memory_order::relaxed);
+      }
+
+      void
+      _M_sub_ssrc() noexcept
+      {
+	_M_value.fetch_sub(_S_ssrc_counter_inc, memory_order::release);
+      }
+
+      // Obtain lock.
+      void
+      _M_lock() noexcept
+      {
+	// Can use relaxed loads to get the current value.
+	// The successful call to _M_try_lock is an acquire operation.
+	auto __old = _M_value.load(memory_order::relaxed);
+	while (!_M_try_lock(__old, memory_order::relaxed))
+	  { }
+      }
+
+      // Precondition: calling thread holds the lock.
+      void
+      _M_unlock() noexcept
+      {
+	_M_value.fetch_sub(_S_locked_bit, memory_order::release);
       }
 
       bool
-      _M_request_stop()
+      _M_request_stop() noexcept
       {
-        bool __stopped = false;
-        if (_M_stopped.compare_exchange_strong(__stopped, true))
-          {
-#ifdef _GLIBCXX_HAS_GTHREADS
-            std::lock_guard<std::mutex> __lck{_M_mtx};
+	// obtain lock and set stop_requested bit
+	auto __old = _M_value.load(memory_order::acquire);
+	do
+	  {
+	    if (__old & _S_stop_requested_bit) // stop request already made
+	      return false;
+	  }
+	while (!_M_try_lock_and_stop(__old));
+
+#if _GLIBCXX_HAS_GTHREADS
+	_M_requester = __gthread_self();
 #endif
-            while (_M_head)
-              {
-                auto __p = _M_head;
-                _M_head = _M_head->_M_next;
-                _Stop_cb::_S_execute(__p);
-              }
-            return true;
-          }
-        return false;
+
+	while (_M_head)
+	  {
+	    bool __last_cb;
+	    _Stop_cb* __cb = _M_head;
+	    _M_head = _M_head->_M_next;
+	    if (_M_head)
+	      {
+		_M_head->_M_prev = nullptr;
+		__last_cb = false;
+	      }
+	    else
+	      __last_cb = true;
+
+	    // Allow other callbacks to be unregistered while __cb runs.
+	    _M_unlock();
+
+	    bool __destroyed = false;
+	    __cb->_M_destroyed = &__destroyed;
+
+	    // run callback
+	    __cb->_M_run();
+
+	    if (!__destroyed)
+	      {
+		__cb->_M_destroyed = nullptr;
+#if _GLIBCXX_HAS_GTHREADS
+		// synchronize with destructor of stop_callback that owns *__cb
+		__cb->_M_done.release();
+#endif
+	      }
+
+	    // Avoid relocking if we already know there are no more callbacks.
+	    if (__last_cb)
+	      return true;
+
+	    _M_lock();
+	  }
+
+	_M_unlock();
+	return true;
       }
 
+      [[__gnu__::__nonnull__]]
       bool
-      _M_register_callback(_Stop_cb* __cb)
+      _M_register_callback(_Stop_cb* __cb) noexcept
       {
-#ifdef _GLIBCXX_HAS_GTHREADS
-        std::lock_guard<std::mutex> __lck{_M_mtx};
-#endif
-        if (_M_stopped)
-          return false;
+	auto __old = _M_value.load(memory_order::acquire);
+	do
+	  {
+	    if (__old & _S_stop_requested_bit) // stop request already made
+	      {
+		__cb->_M_run(); // run synchronously
+		return false;
+	      }
+
+	    if (__old < _S_ssrc_counter_inc) // no stop_source owns *this
+	      // No need to register callback if no stop request can be made.
+	      // Returning false also means the stop_callback does not share
+	      // ownership of this state, but that's not observable.
+	      return false;
+	  }
+	while (!_M_try_lock(__old));
 
         __cb->_M_next = _M_head;
         if (_M_head)
@@ -175,43 +310,163 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
             _M_head->_M_prev = __cb;
           }
         _M_head = __cb;
+	_M_unlock();
         return true;
       }
 
+      // Called by ~stop_callback just before destroying *__cb.
+      [[__gnu__::__nonnull__]]
       void
       _M_remove_callback(_Stop_cb* __cb)
       {
-#ifdef _GLIBCXX_HAS_GTHREADS
-        std::lock_guard<std::mutex> __lck{_M_mtx};
-#endif
+	_M_lock();
+
         if (__cb == _M_head)
           {
             _M_head = _M_head->_M_next;
             if (_M_head)
-              {
-                _M_head->_M_prev = nullptr;
-              }
+	      _M_head->_M_prev = nullptr;
+	    _M_unlock();
+	    return;
           }
-        else if (!__cb->_M_linked())
-          {
-            return;
-          }
-        else
+        else if (__cb->_M_prev)
           {
             __cb->_M_prev->_M_next = __cb->_M_next;
             if (__cb->_M_next)
-              {
-                __cb->_M_next->_M_prev = __cb->_M_prev;
-              }
+	      __cb->_M_next->_M_prev = __cb->_M_prev;
+	    _M_unlock();
+            return;
           }
+
+	_M_unlock();
+
+	// Callback is not in the list, so must have been removed by a call to
+	// _M_request_stop.
+
+#if _GLIBCXX_HAS_GTHREADS
+	// Despite appearances there is no data race on _M_requester. The only
+	// write to it happens before the callback is removed from the list,
+	// and removing it from the list happens before this read.
+	if (!__gthread_equal(_M_requester, __gthread_self()))
+	  {
+	    // Synchronize with completion of callback.
+	    __cb->_M_done.acquire();
+	    // Safe for ~stop_callback to destroy *__cb now.
+	    return;
+	  }
+#endif
+	if (__cb->_M_destroyed)
+	  *__cb->_M_destroyed = true;
+      }
+
+      // Try to obtain the lock.
+      // Returns true if the lock is acquired (with memory order acquire).
+      // Otherwise, sets __curval = _M_value.load(__failure) and returns false.
+      // Might fail spuriously, so must be called in a loop.
+      bool
+      _M_try_lock(value_type& __curval,
+		  memory_order __failure = memory_order::acquire) noexcept
+      {
+	return _M_do_try_lock(__curval, 0, memory_order::acquire, __failure);
+      }
+
+      // Try to obtain the lock to make a stop request.
+      // Returns true if the lock is acquired and the _S_stop_requested_bit is
+      // set (with memory order acq_rel so that other threads see the request).
+      // Otherwise, sets __curval = _M_value.load(memory_order::acquire) and
+      // returns false.
+      // Might fail spuriously, so must be called in a loop.
+      bool
+      _M_try_lock_and_stop(value_type& __curval) noexcept
+      {
+	return _M_do_try_lock(__curval, _S_stop_requested_bit,
+			      memory_order::acq_rel, memory_order::acquire);
+      }
+
+      bool
+      _M_do_try_lock(value_type& __curval, value_type __newbits,
+		     memory_order __success, memory_order __failure) noexcept
+      {
+	if (__curval & _S_locked_bit)
+	  {
+	    _S_yield();
+	    __curval = _M_value.load(__failure);
+	    return false;
+	  }
+	__newbits |= _S_locked_bit;
+	return _M_value.compare_exchange_weak(__curval, __curval | __newbits,
+					      __success, __failure);
       }
     };
 
-    using _Stop_state = std::shared_ptr<_Stop_state_t>;
-    _Stop_state _M_state;
+    struct _Stop_state_ref
+    {
+      _Stop_state_ref() = default;
+
+      explicit
+      _Stop_state_ref(const stop_source&)
+      : _M_ptr(new _Stop_state_t())
+      { }
+
+      _Stop_state_ref(const _Stop_state_ref& __other) noexcept
+      : _M_ptr(__other._M_ptr)
+      {
+	if (_M_ptr)
+	  _M_ptr->_M_add_owner();
+      }
+
+      _Stop_state_ref(_Stop_state_ref&& __other) noexcept
+      : _M_ptr(__other._M_ptr)
+      {
+	__other._M_ptr = nullptr;
+      }
+
+      _Stop_state_ref&
+      operator=(const _Stop_state_ref& __other) noexcept
+      {
+	if (auto __ptr = __other._M_ptr; __ptr != _M_ptr)
+	  {
+	    if (__ptr)
+	      __ptr->_M_add_owner();
+	    if (_M_ptr)
+	      _M_ptr->_M_release_ownership();
+	    _M_ptr = __ptr;
+	  }
+	return *this;
+      }
+
+      _Stop_state_ref&
+      operator=(_Stop_state_ref&& __other) noexcept
+      {
+	_Stop_state_ref(std::move(__other)).swap(*this);
+	return *this;
+      }
+
+      ~_Stop_state_ref()
+      {
+	if (_M_ptr)
+	  _M_ptr->_M_release_ownership();
+      }
+
+      void
+      swap(_Stop_state_ref& __other) noexcept
+      { std::swap(_M_ptr, __other._M_ptr); }
+
+      explicit operator bool() const noexcept { return _M_ptr != nullptr; }
+
+      _Stop_state_t* operator->() const noexcept { return _M_ptr; }
+
+      friend bool
+      operator==(const _Stop_state_ref&, const _Stop_state_ref&) = default;
+
+    private:
+      _Stop_state_t* _M_ptr = nullptr;
+    };
+
+    _Stop_state_ref _M_state;
 
     explicit
-    stop_token(const _Stop_state& __state) noexcept
+    stop_token(const _Stop_state_ref& __state) noexcept
     : _M_state{__state}
     { }
   };
@@ -220,34 +475,41 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class stop_source
   {
   public:
-    stop_source()
-      : _M_state(std::make_shared<stop_token::_Stop_state_t>())
+    stop_source() : _M_state(*this)
     { }
 
     explicit stop_source(std::nostopstate_t) noexcept
     { }
 
     stop_source(const stop_source& __other) noexcept
-      : _M_state(__other._M_state)
-    { }
+    : _M_state(__other._M_state)
+    {
+      if (_M_state)
+	_M_state->_M_add_ssrc();
+    }
 
-    stop_source(stop_source&& __other) noexcept
-      : _M_state(std::move(__other._M_state))
-    { }
+    stop_source(stop_source&&) noexcept = default;
 
     stop_source&
-    operator=(const stop_source& __rhs) noexcept
+    operator=(const stop_source& __other) noexcept
     {
-      if (_M_state != __rhs._M_state)
-        _M_state = __rhs._M_state;
+      if (_M_state != __other._M_state)
+	{
+	  stop_source __sink(std::move(*this));
+	  _M_state = __other._M_state;
+	  if (_M_state)
+	    _M_state->_M_add_ssrc();
+	}
       return *this;
     }
 
     stop_source&
-    operator=(stop_source&& __rhs) noexcept
+    operator=(stop_source&&) noexcept = default;
+
+    ~stop_source()
     {
-      std::swap(_M_state, __rhs._M_state);
-      return *this;
+      if (_M_state)
+	_M_state->_M_sub_ssrc();
     }
 
     [[nodiscard]]
@@ -261,7 +523,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     bool
     stop_requested() const noexcept
     {
-      return stop_possible() && _M_state->_M_stop_requested();
+      return static_cast<bool>(_M_state) && _M_state->_M_stop_requested();
     }
 
     bool
@@ -299,14 +561,16 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
   private:
-    stop_token::_Stop_state _M_state;
+    stop_token::_Stop_state_ref _M_state;
   };
 
   /// A wrapper for callbacks to be run when a stop request is made.
   template<typename _Callback>
     class [[nodiscard]] stop_callback
-      : private stop_token::_Stop_cb
     {
+      static_assert(is_nothrow_destructible_v<_Callback>);
+      static_assert(is_invocable_v<_Callback>);
+
     public:
       using callback_type = _Callback;
 
@@ -315,13 +579,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
         explicit
 	stop_callback(const stop_token& __token, _Cb&& __cb)
         noexcept(is_nothrow_constructible_v<_Callback, _Cb>)
-        : _Stop_cb(&_S_execute), _M_cb(std::forward<_Cb>(__cb))
+        : _M_cb(std::forward<_Cb>(__cb))
         {
 	  if (auto __state = __token._M_state)
 	    {
-	      if (__state->_M_stop_requested())
-		_S_execute(this); // ensures std::terminate on throw
-	      else if (__state->_M_register_callback(this))
+	      if (__state->_M_register_callback(&_M_cb))
 		_M_state.swap(__state);
 	    }
         }
@@ -331,13 +593,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
         explicit
 	stop_callback(stop_token&& __token, _Cb&& __cb)
         noexcept(is_nothrow_constructible_v<_Callback, _Cb>)
-        : _Stop_cb(&_S_execute), _M_cb(std::forward<_Cb>(__cb))
+        : _M_cb(std::forward<_Cb>(__cb))
 	{
 	  if (auto& __state = __token._M_state)
 	    {
-	      if (__state->_M_stop_requested())
-		_S_execute(this); // ensures std::terminate on throw
-	      else if (__state->_M_register_callback(this))
+	      if (__state->_M_register_callback(&_M_cb))
 		_M_state.swap(__state);
 	    }
 	}
@@ -346,7 +606,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
 	if (_M_state)
 	  {
-	    _M_state->_M_remove_callback(this);
+	    _M_state->_M_remove_callback(&_M_cb);
 	  }
       }
 
@@ -356,14 +616,28 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       stop_callback& operator=(stop_callback&&) = delete;
 
     private:
-      _Callback _M_cb;
-      stop_token::_Stop_state _M_state = nullptr;
-
-      static void
-      _S_execute(_Stop_cb* __that) noexcept
+      struct _Cb_impl : stop_token::_Stop_cb
       {
-	static_cast<stop_callback*>(__that)->_M_cb();
-      }
+	template<typename _Cb>
+	  explicit
+	  _Cb_impl(_Cb&& __cb)
+	  : _Stop_cb(&_S_execute),
+	    _M_cb(std::forward<_Cb>(__cb))
+	  { }
+
+	_Callback _M_cb;
+
+	[[__gnu__::__nonnull__]]
+	static void
+	_S_execute(_Stop_cb* __that) noexcept
+	{
+	  _Callback& __cb = static_cast<_Cb_impl*>(__that)->_M_cb;
+	  std::forward<_Callback>(__cb)();
+	}
+      };
+
+      _Cb_impl _M_cb;
+      stop_token::_Stop_state_ref _M_state;
     };
 
   template<typename _Callback>
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock-mt.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock-mt.cc
new file mode 100644
index 00000000000..12c54db554f
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock-mt.cc
@@ -0,0 +1,50 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a -pthread"  }
+// { dg-require-effective-target c++2a }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+#include <stop_token>
+#include <memory>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::stop_source ssrc;
+  std::stop_token stok = ssrc.get_token();
+  using F = void(*)();
+  std::unique_ptr<std::stop_callback<F>> pcb;
+  auto dereg = [&pcb] { pcb.reset(); };
+  std::stop_callback cb1(stok, dereg);
+  pcb = std::make_unique<std::stop_callback<F>>(stok, []{});
+  std::stop_callback cb2(stok, dereg);
+
+  // PR libstdc++/92895
+  // Making a stop request runs the callbacks. Whichever of cb1 and cb2
+  // runs first will destroy *pcb, which will try to unregister it.
+  // This recursive access to the shared stop state within a callback must
+  // work without deadlock.
+  ssrc.request_stop();
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock.cc
new file mode 100644
index 00000000000..f9de6e02562
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock.cc
@@ -0,0 +1,48 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <stop_token>
+#include <memory>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::stop_source ssrc;
+  std::stop_token stok = ssrc.get_token();
+  using F = void(*)();
+  std::unique_ptr<std::stop_callback<F>> pcb;
+  auto dereg = [&pcb] { pcb.reset(); };
+  std::stop_callback cb1(stok, dereg);
+  pcb = std::make_unique<std::stop_callback<F>>(stok, []{});
+  std::stop_callback cb2(stok, dereg);
+
+  // PR libstdc++/92895
+  // Making a stop request runs the callbacks. Whichever of cb1 and cb2
+  // runs first will destroy *pcb, which will try to unregister it.
+  // This recursive access to the shared stop state within a callback must
+  // work without deadlock.
+  ssrc.request_stop();
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destroy.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destroy.cc
new file mode 100644
index 00000000000..3fa4d21c55c
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destroy.cc
@@ -0,0 +1,83 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a -pthread"  }
+// { dg-require-effective-target c++2a }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+#include <stop_token>
+#include <atomic>
+#include <thread>
+#include <testsuite_hooks.h>
+
+struct F
+{
+  static std::atomic<int> stage;
+
+  F(int) { }
+
+  ~F()
+  {
+    // PR libstdc++/92895
+    // Callback function must not be destroyed while still executing.
+    VERIFY( stage == 4 );
+  }
+
+  void operator()() const noexcept
+  {
+    stage = 2; // trigger destructor of stop_callback that owns *this
+    while (stage == 2)
+      std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    std::this_thread::sleep_for(std::chrono::milliseconds(500));
+    stage = 4; // destructor checks for this
+  }
+};
+
+std::atomic<int> F::stage{0};
+
+void
+test01()
+{
+  std::stop_source ssrc;
+  std::stop_token stok = ssrc.get_token();
+  std::thread t1([&ssrc] {
+    while (F::stage == 0)
+      std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    ssrc.request_stop();
+    while (F::stage != 5)
+      std::this_thread::sleep_for(std::chrono::milliseconds(10));
+  });
+
+  std::thread t2([&ssrc] {
+    std::stop_callback<F> cb(ssrc.get_token(), 0);
+    F::stage = 1; // trigger stop request in other thread, which runs callback
+    while (F::stage == 1)
+      std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    F::stage = 3;
+    // stop_callback destructor should block until callback completes
+  });
+
+  t2.join();
+  F::stage = 5; // allow first thread to exit
+  t1.join();
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destructible_neg.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destructible_neg.cc
new file mode 100644
index 00000000000..5016af009c0
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destructible_neg.cc
@@ -0,0 +1,57 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <stop_token>
+
+struct F
+{
+  F();
+
+  void operator()() const { }
+
+private:
+  ~F();
+};
+
+auto
+test01(std::stop_token& tok, F& f)
+{
+  auto ok = sizeof(std::stop_callback<F&>);
+  auto bad = sizeof(std::stop_callback<F>); // { dg-error "here" }
+  return ok + bad;
+}
+
+struct G
+{
+  G();
+  ~G() noexcept(false);
+
+  void operator()() const { }
+};
+
+auto
+test02(std::stop_token& tok, G& g)
+{
+  auto ok = sizeof(std::stop_callback<G&>);
+  auto bad = sizeof(std::stop_callback<G>); // { dg-error "here" }
+  return ok + bad;
+}
+
+// { dg-error "static assertion failed" "" { target *-*-* } 0 }
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invocable_neg.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invocable_neg.cc
new file mode 100644
index 00000000000..459a481ed6e
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invocable_neg.cc
@@ -0,0 +1,35 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <stop_token>
+
+struct F
+{
+};
+
+auto
+test01(std::stop_token& tok, F& f)
+{
+  auto bad1 = sizeof(std::stop_callback<F&>); // { dg-error "here" }
+  auto bad2 = sizeof(std::stop_callback<F>);  // { dg-error "here" }
+  return bad1 + bad2;
+}
+
+// { dg-error "static assertion failed" "" { target *-*-* } 0 }
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invoke.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invoke.cc
new file mode 100644
index 00000000000..9b8137cc46d
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invoke.cc
@@ -0,0 +1,62 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <stop_token>
+#include <testsuite_hooks.h>
+
+int lval[5];
+int rval[5];
+
+void
+test01()
+{
+  std::stop_source ssrc;
+  std::stop_token stok = ssrc.get_token();
+  struct F
+  {
+    void operator()() const & { ++lval[i]; }
+    void operator()() && { ++rval[i]; }
+
+    int i;
+  };
+  std::stop_callback<F> cb0(stok, F{0});
+  std::stop_callback<F> cb1(stok, F{1});
+  std::stop_callback<F> cb2(stok, F{2});
+  F f3{3};
+  std::stop_callback<F&> cb3(stok, f3);
+  std::stop_callback<const F> cb4(stok, F{4});
+
+  // PR libstdc++/92895
+  // Callback should be invoked with correct value category.
+  ssrc.request_stop();
+
+  VERIFY( lval[0] == 0 && lval[1] == 0 && lval[2] == 0 );
+  VERIFY( lval[3] == 1 );
+  VERIFY( lval[4] == 1 );
+  VERIFY( rval[0] == 1 );
+  VERIFY( rval[1] == 1 );
+  VERIFY( rval[2] == 1 );
+  VERIFY( rval[3] == 0 && rval[4] == 0 );
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_source/assign.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_source/assign.cc
new file mode 100644
index 00000000000..c822e8e398f
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_source/assign.cc
@@ -0,0 +1,51 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <stop_token>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::stop_source src1, src2;
+  const std::stop_source orig1(src1);
+  VERIFY( src1 != src2 );
+  src1 = src2;
+  VERIFY( src1 == src2 );
+  VERIFY( src1 != orig1 );
+}
+
+void
+test02()
+{
+  std::stop_source src1, src2;
+  const std::stop_source orig1(src1), orig2(src2), src0(std::nostopstate);
+  src1 = std::move(src2);
+  VERIFY( src1 == orig2 );
+  VERIFY( src2 == src0 );
+  VERIFY( src1 != orig1 );
+  VERIFY( src0 != orig1 );
+}
+
+int main()
+{
+  test01();
+  test02();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_token/stop_possible.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_token/stop_possible.cc
new file mode 100644
index 00000000000..ee8de6889ed
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_token/stop_possible.cc
@@ -0,0 +1,49 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <stop_token>
+#include <memory>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::stop_source ssrc;
+  std::stop_token tok = ssrc.get_token();
+  VERIFY(tok.stop_possible());
+
+  ssrc.request_stop();
+  VERIFY(tok.stop_possible());
+}
+
+void
+test02()
+{
+  std::stop_token tok = std::stop_source().get_token();
+  // PR libstdc++/92895
+  // stop_possible() is false when there is no associated stop_source
+  VERIFY(!tok.stop_possible());
+}
+
+int main()
+{
+  test01();
+  test02();
+}