From 0a8f4febf75e9e44f847b65776d7f5f38940b3bf Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Thu, 23 Jan 2020 16:46:17 +0000 Subject: [PATCH] libstdc++: Fix conformance issues in (PR92895) Fix synchronization issues in . 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. --- libstdc++-v3/ChangeLog | 69 +++ libstdc++-v3/include/std/stop_token | 478 ++++++++++++++---- .../stop_token/stop_callback/deadlock-mt.cc | 50 ++ .../stop_token/stop_callback/deadlock.cc | 48 ++ .../stop_token/stop_callback/destroy.cc | 83 +++ .../stop_callback/destructible_neg.cc | 57 +++ .../stop_token/stop_callback/invocable_neg.cc | 35 ++ .../stop_token/stop_callback/invoke.cc | 62 +++ .../stop_token/stop_source/assign.cc | 51 ++ .../stop_token/stop_token/stop_possible.cc | 49 ++ 10 files changed, 880 insertions(+), 102 deletions(-) create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock-mt.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/deadlock.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destroy.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/destructible_neg.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invocable_neg.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_callback/invoke.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_source/assign.cc create mode 100644 libstdc++-v3/testsuite/30_threads/stop_token/stop_token/stop_possible.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index e9a8a677e4c..6953a789bc3 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,74 @@ 2020-01-29 Jonathan Wakely + 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. + * libsupc++/compare (__detail::__3way_builtin_ptr_cmp): Use three_way_comparable_with. (__detail::__3way_cmp_with): Remove workaround for fixed bug. diff --git a/libstdc++-v3/include/std/stop_token b/libstdc++-v3/include/std/stop_token index e23d139e66c..6fb8ae05197 100644 --- a/libstdc++-v3/include/std/stop_token +++ b/libstdc++-v3/include/std/stop_token @@ -32,13 +32,13 @@ #if __cplusplus > 201703L #include -#include -#include -#include -#include #ifdef _GLIBCXX_HAS_GTHREADS # define __cpp_lib_jthread 201907L +# include +# if __has_include() +# include +# 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(_M_state); + return static_cast(_M_state) && _M_state->_M_stop_possible(); } [[nodiscard]] bool stop_requested() const noexcept { - return stop_possible() && _M_state->_M_stop_requested(); + return static_cast(_M_state) && _M_state->_M_stop_requested(); } void @@ -98,76 +100,209 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template friend class stop_callback; - struct _Stop_cb + static void + _S_yield() noexcept { - void(*_M_callback)(_Stop_cb*); - _Stop_cb* _M_prev = nullptr; - _Stop_cb* _M_next = nullptr; +#if defined __i386__ || defined __x86_64__ + __builtin_ia32_pause(); +#elif defined _GLIBCXX_USE_SCHED_YIELD + __gthread_yield(); +#endif + } - template - _Stop_cb(_Cb&& __cb) - : _M_callback(std::forward<_Cb>(__cb)) - { } +#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) { } - bool - _M_linked() const noexcept - { - return (_M_prev != nullptr) - || (_M_next != nullptr); - } + void release() { _M_counter.fetch_add(1, memory_order::release); } - static void - _S_execute(_Stop_cb* __cb) noexcept + void acquire() { - __cb->_M_callback(__cb); - __cb->_M_prev = __cb->_M_next = nullptr; + int __old = 1; + while (!_M_counter.compare_exchange_weak(__old, 0, + memory_order::acquire, + memory_order::relaxed)) + { + __old = 1; + _S_yield(); + } } + + atomic _M_counter; + }; +#endif + + struct _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}; + + [[__gnu__::__nonnull__]] + explicit + _Stop_cb(__cb_type* __cb) + : _M_callback(__cb) + { } + + void _M_run() noexcept { _M_callback(this); } }; struct _Stop_state_t { - std::atomic _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 _M_owners{1}; + std::atomic _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 __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 __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 __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); + } + }; + + 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; }; - using _Stop_state = std::shared_ptr<_Stop_state_t>; - _Stop_state _M_state; + _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_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(_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 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(__that)->_M_cb(); - } + template + 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 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 +// . + +// { dg-options "-std=gnu++2a -pthread" } +// { dg-require-effective-target c++2a } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } + +#include +#include +#include + +void +test01() +{ + std::stop_source ssrc; + std::stop_token stok = ssrc.get_token(); + using F = void(*)(); + std::unique_ptr> pcb; + auto dereg = [&pcb] { pcb.reset(); }; + std::stop_callback cb1(stok, dereg); + pcb = std::make_unique>(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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do run { target c++2a } } + +#include +#include +#include + +void +test01() +{ + std::stop_source ssrc; + std::stop_token stok = ssrc.get_token(); + using F = void(*)(); + std::unique_ptr> pcb; + auto dereg = [&pcb] { pcb.reset(); }; + std::stop_callback cb1(stok, dereg); + pcb = std::make_unique>(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 +// . + +// { dg-options "-std=gnu++2a -pthread" } +// { dg-require-effective-target c++2a } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } + +#include +#include +#include +#include + +struct F +{ + static std::atomic 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 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 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include + +struct F +{ + F(); + + void operator()() const { } + +private: + ~F(); +}; + +auto +test01(std::stop_token& tok, F& f) +{ + auto ok = sizeof(std::stop_callback); + auto bad = sizeof(std::stop_callback); // { 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); + auto bad = sizeof(std::stop_callback); // { 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include + +struct F +{ +}; + +auto +test01(std::stop_token& tok, F& f) +{ + auto bad1 = sizeof(std::stop_callback); // { dg-error "here" } + auto bad2 = sizeof(std::stop_callback); // { 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do run { target c++2a } } + +#include +#include + +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 cb0(stok, F{0}); + std::stop_callback cb1(stok, F{1}); + std::stop_callback cb2(stok, F{2}); + F f3{3}; + std::stop_callback cb3(stok, f3); + std::stop_callback 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do run { target c++2a } } + +#include +#include + +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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do run { target c++2a } } + +#include +#include +#include + +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(); +} -- 2.30.2