On 15/11/19 14:40 +0000, Jonathan Wakely wrote:
On 15/11/19 14:38 +0000, Jonathan Wakely wrote:
On 13/11/19 17:59 -0800, Thomas Rodgers wrote:
+ // TODO verify the standard requires this
+#if 0
+ // register another callback
+ bool cb3called{false};
+ std::stop_callback scb3{stok, [&]
+ {
+ cb3called = true;
+ }};
+ VERIFY(ssrc.stop_possible());
+ VERIFY(ssrc.stop_requested());
+ VERIFY(stok.stop_possible());
+ VERIFY(stok.stop_requested());
+ VERIFY(!cb1called);
+ VERIFY(cb2called);
+ VERIFY(cb3called);
+#endif
The working draft definitely requires this:
Effects: Initializes callback with std::forward<C>(cb). If st.stop_requested()
is true, then
std::forward<Callback>(callback)() is evaluated in the current thread before
the constructor
returns.
I've committed this fix to the nostopstate initializer, so it defines
a variable, instead of declaring a function. I've also added a test
that uses the stop_source(nostopstate_t) constructor, and a few other
tweaks to include the new header where it's needed.
This fixes some other issues I noticed, and should fix the failues for
the single-threaded multilib on AIX.
Tested powerpc64le-linux, committed to trunk.
commit 9c9f5a4bd84b924190551b65d4b66665aaa3fbe6
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri Nov 15 21:27:26 2019 +0000
libstdc++: Fix <stop_token> and improve tests
* include/std/stop_token: Reduce header dependencies by including
internal headers.
(stop_token::swap(stop_token&), swap(stop_token&, stop_token&)):
Define.
(operator!=(const stop_token&, const stop_token&)): Fix return value.
(stop_token::_Stop_cb::_Stop_cb(Cb&&)): Use std::forward instead of
(stop_token::_Stop_state_t) [_GLIBCXX_HAS_GTHREADS]: Use lock_guard
instead of unique_lock.
[!_GLIBCXX_HAS_GTHREADS]: Do not use mutex.
(stop_token::stop_token(_Stop_state)): Change parameter to lvalue
reference.
(stop_source): Remove unnecessary using-declarations for names only
used once.
(swap(stop_source&, stop_source&)): Define.
(stop_callback(const stop_token&, _Cb&&))
(stop_callback(stop_token&&, _Cb&&)): Replace lambdas with a named
function. Use std::forward instead of std::move. Run callbacks if a
stop request has already been made.
(stop_source::_M_execute()): Remove.
(stop_source::_S_execute(_Stop_cb*)): Define.
* include/std/version (__cpp_lib_jthread): Define conditionally.
* testsuite/30_threads/stop_token/stop_callback.cc: New test.
* testsuite/30_threads/stop_token/stop_source.cc: New test.
* testsuite/30_threads/stop_token/stop_token.cc: Enable test for
immediate execution of callback.
diff --git a/libstdc++-v3/include/std/stop_token b/libstdc++-v3/include/std/stop_token
index f96c267f22e..bb082431d7f 100644
--- a/libstdc++-v3/include/std/stop_token
+++ b/libstdc++-v3/include/std/stop_token
@@ -31,24 +31,25 @@
#if __cplusplus > 201703L
-#include <type_traits>
-#include <memory>
-#include <mutex>
#include <atomic>
+#include <bits/std_mutex.h>
+#include <ext/concurrence.h>
+#include <bits/unique_ptr.h>
+#include <bits/shared_ptr.h>
-#define __cpp_lib_jthread 201907L
+#ifdef _GLIBCXX_HAS_GTHREADS
+# define __cpp_lib_jthread 201907L
+#endif
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
- class stop_source;
- template<typename _Callback>
- class stop_callback;
-
+ /// Tag type indicating a stop_source should have no shared-stop-state.
struct nostopstate_t { explicit nostopstate_t() = default; };
inline constexpr nostopstate_t nostopstate{};
+ /// Allow testing whether a stop request has been made on a `stop_source`.
class stop_token
{
public:
@@ -63,7 +64,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
operator=(const stop_token& __rhs) noexcept = default;
stop_token&
- operator=(stop_token&& __rhs) noexcept;
+ operator=(stop_token&& __rhs) noexcept = default;
[[nodiscard]]
bool
@@ -79,6 +80,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return stop_possible() && _M_state->_M_stop_requested();
}
+ void
+ swap(stop_token& __rhs) noexcept
+ { _M_state.swap(__rhs._M_state); }
+
[[nodiscard]]
friend bool
operator==(const stop_token& __a, const stop_token& __b)
@@ -90,26 +95,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
friend bool
operator!=(const stop_token& __a, const stop_token& __b)
{
- return __a._M_state == __b._M_state;
+ return __a._M_state != __b._M_state;
}
- private:
- friend stop_source;
- template<typename _Callback>
- friend class stop_callback;
+ friend void
+ swap(stop_token& __lhs, stop_token& __rhs) noexcept
+ { __lhs.swap(__rhs); }
- struct _Stop_cb {
+ private:
+ friend class stop_source;
+ template<typename _Callback>
+ friend class stop_callback;
+
+ struct _Stop_cb
+ {
void(*_M_callback)(_Stop_cb*);
_Stop_cb* _M_prev = nullptr;
_Stop_cb* _M_next = nullptr;
template<typename _Cb>
- _Stop_cb(_Cb&& __cb)
- : _M_callback(std::move(__cb))
- { }
+ _Stop_cb(_Cb&& __cb)
+ : _M_callback(std::forward<_Cb>(__cb))
+ { }
bool
- _M_linked() const
+ _M_linked() const noexcept
{
return (_M_prev != nullptr)
|| (_M_next != nullptr);
@@ -123,17 +133,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
};
- struct _Stop_state_t {
- std::atomic<bool> _M_stopped;
- std::mutex _M_mtx;
+ struct _Stop_state_t
+ {
+ std::atomic<bool> _M_stopped{false};
_Stop_cb* _M_head = nullptr;
+#ifdef _GLIBCXX_HAS_GTHREADS
+ std::mutex _M_mtx;
+#endif
- _Stop_state_t()
- : _M_stopped{false}
- { }
+ _Stop_state_t() = default;
bool
- _M_stop_requested()
+ _M_stop_requested() noexcept
{
return _M_stopped;
}
@@ -144,7 +155,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
bool __stopped = false;
if (_M_stopped.compare_exchange_strong(__stopped, true))
{
- std::unique_lock<std::mutex> __lck{_M_mtx};
+#ifdef _GLIBCXX_HAS_GTHREADS
+ std::lock_guard<std::mutex> __lck{_M_mtx};
+#endif
while (_M_head)
{
auto __p = _M_head;
@@ -159,7 +172,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
bool
_M_register_callback(_Stop_cb* __cb)
{
- std::unique_lock<std::mutex> __lck{_M_mtx};
+#ifdef _GLIBCXX_HAS_GTHREADS
+ std::lock_guard<std::mutex> __lck{_M_mtx};
+#endif
if (_M_stopped)
return false;
@@ -169,13 +184,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_head->_M_prev = __cb;
}
_M_head = __cb;
- return true;
+ return true;
}
void
_M_remove_callback(_Stop_cb* __cb)
{
- std::unique_lock<std::mutex> __lck{_M_mtx};
+#ifdef _GLIBCXX_HAS_GTHREADS
+ std::lock_guard<std::mutex> __lck{_M_mtx};
+#endif
if (__cb == _M_head)
{
_M_head = _M_head->_M_next;
@@ -202,18 +219,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using _Stop_state = std::shared_ptr<_Stop_state_t>;
_Stop_state _M_state;
- explicit stop_token(_Stop_state __state)
- : _M_state{std::move(__state)}
+ explicit
+ stop_token(const _Stop_state& __state) noexcept
+ : _M_state{__state}
{ }
};
- class stop_source {
- using _Stop_state_t = stop_token::_Stop_state_t;
- using _Stop_state = stop_token::_Stop_state;
-
+ /// A type that allows a stop request to be made.
+ class stop_source
+ {
public:
stop_source()
- : _M_state(std::make_shared<_Stop_state_t>())
+ : _M_state(std::make_shared<stop_token::_Stop_state_t>())
{ }
explicit stop_source(std::nostopstate_t) noexcept
@@ -274,7 +291,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void
swap(stop_source& __other) noexcept
{
- std::swap(_M_state, __other._M_state);
+ _M_state.swap(__other._M_state);
}
[[nodiscard]]
@@ -291,59 +308,63 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return __a._M_state != __b._M_state;
}
+ friend void
+ swap(stop_source& __lhs, stop_source& __rhs) noexcept
+ {
+ __lhs.swap(__rhs);
+ }
+
private:
- _Stop_state _M_state;
+ stop_token::_Stop_state _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
{
- using _Stop_cb = stop_token::_Stop_cb;
- using _Stop_state = stop_token::_Stop_state;
public:
using callback_type = _Callback;
template<typename _Cb,
- std::enable_if_t<std::is_constructible_v<_Callback, _Cb>, int> = 0>
- explicit stop_callback(const stop_token& __token, _Cb&& __cb)
- noexcept(std::is_nothrow_constructible_v<_Callback, _Cb>)
- : _Stop_cb([](_Stop_cb* __that) noexcept
- {
- static_cast<stop_callback*>(__that)->_M_execute();
- }),
- _M_cb(std::move(__cb))
+ enable_if_t<is_constructible_v<_Callback, _Cb>, int> = 0>
+ 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))
{
- auto res = __token._M_state->_M_register_callback(this);
- if (__token._M_state && res)
- {
- _M_state = __token._M_state;
- }
+ 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))
+ _M_state.swap(__state);
+ }
}
template<typename _Cb,
- std::enable_if_t<std::is_constructible_v<_Callback, _Cb>, int> = 0>
- explicit stop_callback(stop_token&& __token, _Cb&& __cb)
- noexcept(std::is_nothrow_constructible_v<_Callback, _Cb>)
- : _Stop_cb([](_Stop_cb* __that) noexcept
- {
- static_cast<stop_callback*>(__that)->_M_execute();
- }),
- _M_cb(std::move(__cb))
- {
- if (__token._M_state && __token._M_state->_M_register_callback(this))
- {
- std::swap(_M_state, __token._M_state);
- }
- }
+ enable_if_t<is_constructible_v<_Callback, _Cb>, int> = 0>
+ 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))
+ {
+ 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))
+ _M_state.swap(__state);
+ }
+ }
~stop_callback()
- {
- if (_M_state)
- {
- _M_state->_M_remove_callback(this);
- }
- }
+ {
+ if (_M_state)
+ {
+ _M_state->_M_remove_callback(this);
+ }
+ }
stop_callback(const stop_callback&) = delete;
stop_callback& operator=(const stop_callback&) = delete;
@@ -352,12 +373,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
private:
_Callback _M_cb;
- _Stop_state _M_state = nullptr;
+ stop_token::_Stop_state _M_state = nullptr;
- void
- _M_execute() noexcept
+ static void
+ _S_execute(_Stop_cb* __that) noexcept
{
- _M_cb();
+ static_cast<stop_callback*>(__that)->_M_cb();
}
};
@@ -366,5 +387,5 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
-#endif // __cplusplus >= 201703L
+#endif // __cplusplus > 201703L
#endif // _GLIBCXX_STOP_TOKEN
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index 27807428903..8e08dc99049 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -184,10 +184,12 @@
#define __cpp_lib_constexpr_invoke 201907L
#define __cpp_lib_erase_if 201900L
#define __cpp_lib_interpolate 201902L
+#ifdef _GLIBCXX_HAS_GTHREADS
+# define __cpp_lib_jthread 201907L
+#endif
#define __cpp_lib_list_remove_return_type 201806L
#define __cpp_lib_math_constants 201907L
#define __cpp_lib_span 201902L
-#define __cpp_lib_jthread 201907L
#if __cpp_impl_three_way_comparison >= 201907L
# define __cpp_lib_three_way_comparison 201711L
#endif
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback.cc
new file mode 100644
index 00000000000..beb155d494a
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_callback.cc
@@ -0,0 +1,128 @@
+// Copyright (C) 2019 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 <functional>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+ bool called = false;
+ std::function<void()> f = [&called]{ called = true; };
+ std::stop_source ssrc;
+ ssrc.request_stop();
+ std::stop_token tok = ssrc.get_token();
+ std::stop_callback cb1(tok, f);
+ VERIFY( tok.stop_possible() );
+ VERIFY( f != nullptr );
+ VERIFY( called == true );
+
+ called = false;
+ std::stop_callback cb2(std::move(tok), f);
+ // when callback is executed immediately, no change in ownership:
+ VERIFY( tok.stop_possible() );
+ VERIFY( f != nullptr );
+ VERIFY( called == true );
+
+ std::stop_token sink(std::move(tok)); // leave tok empty
+
+ called = false;
+ std::stop_callback cb3(tok, f);
+ VERIFY( f != nullptr );
+ VERIFY( called == false );
+
+ called = false;
+ std::stop_callback cb4(std::move(tok), f);
+ VERIFY( f != nullptr );
+ VERIFY( called == false );
+}
+
+void
+test02()
+{
+ bool called = false;
+ std::function<void()> f0 = [&called]{ called = true; };
+ std::function<void()> f = f0;
+ std::stop_source ssrc;
+ ssrc.request_stop();
+ std::stop_token tok = ssrc.get_token();
+
+ std::stop_callback cb1(tok, std::move(f));
+ VERIFY( tok.stop_possible() );
+ VERIFY( f == nullptr );
+ VERIFY( called == true );
+
+ called = false;
+ f = f0;
+ std::stop_callback cb2(std::move(tok), std::move(f));
+ // when callback is executed immediately, no change in ownership:
+ VERIFY( tok.stop_possible() );
+ VERIFY( f == nullptr );
+ VERIFY( called == true );
+
+ std::stop_token sink(std::move(tok)); // leave tok empty
+
+ called = false;
+ f = f0;
+ std::stop_callback cb3(tok, std::move(f));
+ VERIFY( f == nullptr );
+ VERIFY( called == false );
+
+ called = false;
+ f = f0;
+ std::stop_callback cb4(std::move(tok), std::move(f));
+ VERIFY( f == nullptr );
+ VERIFY( called == false );
+}
+
+void
+test03()
+{
+ bool called[4] = { };
+ std::stop_source ssrc;
+ std::stop_token tok = ssrc.get_token();
+ std::stop_callback cb1(tok, [&]{ called[0] = true; });
+ VERIFY( tok.stop_possible() );
+ VERIFY( called[0] == false );
+
+ std::stop_callback cb2(std::move(tok), [&]{ called[1] = true; });
+ VERIFY( !tok.stop_possible() );
+ VERIFY( called[1] == false );
+
+ std::stop_callback cb3(tok, [&]{ called[2] = true; });
+ VERIFY( called[2] == false );
+
+ std::stop_callback cb4(std::move(tok), [&]{ called[3] = true; });
+ VERIFY( called[3] == false );
+
+ ssrc.request_stop();
+ VERIFY( called[0] == true );
+ VERIFY( called[1] == true );
+ VERIFY( called[2] == false );
+ VERIFY( called[3] == false );
+}
+
+int main()
+{
+ test01();
+ test02();
+ test03();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_source.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_source.cc
index 9e477b7d7ee..3fe76143bd0 100644
--- a/libstdc++-v3/testsuite/30_threads/stop_token/stop_source.cc
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_source.cc
@@ -68,8 +68,27 @@ test02()
VERIFY( !ssrc.stop_requested() );
}
+void
+test03()
+{
+ std::stop_source s1;
+ s1.request_stop();
+ std::stop_source s2(std::nostopstate);
+ s1.swap(s2);
+ VERIFY( !s1.stop_possible() );
+ VERIFY( !s1.stop_requested() );
+ VERIFY( s2.stop_possible() );
+ VERIFY( s2.stop_requested() );
+ swap(s1, s2);
+ VERIFY( s1.stop_possible() );
+ VERIFY( s1.stop_requested() );
+ VERIFY( !s2.stop_possible() );
+ VERIFY( !s2.stop_requested() );
+}
+
int main()
{
test01();
test02();
+ test03();
}
diff --git a/libstdc++-v3/testsuite/30_threads/stop_token/stop_token.cc b/libstdc++-v3/testsuite/30_threads/stop_token/stop_token.cc
index f0772c1d80f..6dc850c6441 100644
--- a/libstdc++-v3/testsuite/30_threads/stop_token/stop_token.cc
+++ b/libstdc++-v3/testsuite/30_threads/stop_token/stop_token.cc
@@ -21,7 +21,76 @@
#include <stop_token>
#include <testsuite_hooks.h>
-int main()
+void
+test01()
+{
+ std::stop_source ssrc;
+ std::stop_token tok = ssrc.get_token();
+ VERIFY(tok.stop_possible());
+ VERIFY(!tok.stop_requested());
+
+ std::stop_token copy(tok);
+ VERIFY(copy == tok);
+ VERIFY(copy.stop_possible());
+ VERIFY(!copy.stop_requested());
+ VERIFY(tok.stop_possible());
+ VERIFY(!tok.stop_requested());
+
+ std::stop_token move(std::move(tok));
+ VERIFY(move != tok);
+ VERIFY(move == copy);
+ VERIFY(move.stop_possible());
+ VERIFY(!move.stop_requested());
+ VERIFY(!tok.stop_possible());
+ VERIFY(!tok.stop_requested());
+ VERIFY(copy.stop_possible());
+ VERIFY(!copy.stop_requested());
+
+ ssrc.request_stop();
+ VERIFY(move.stop_possible());
+ VERIFY(move.stop_requested());
+ VERIFY(!tok.stop_possible());
+ VERIFY(!tok.stop_requested());
+ VERIFY(copy.stop_possible());
+ VERIFY(copy.stop_requested());
+
+ tok.swap(move);
+ VERIFY(tok == copy);
+ VERIFY(!move.stop_possible());
+ VERIFY(!move.stop_requested());
+ VERIFY(tok.stop_possible());
+ VERIFY(tok.stop_requested());
+ VERIFY(copy.stop_possible());
+ VERIFY(copy.stop_requested());
+
+ swap(move, copy);
+ VERIFY(tok == move);
+ VERIFY(move.stop_possible());
+ VERIFY(move.stop_requested());
+ VERIFY(tok.stop_possible());
+ VERIFY(tok.stop_requested());
+ VERIFY(!copy.stop_possible());
+ VERIFY(!copy.stop_requested());
+}
+
+void
+test02()
+{
+ std::stop_source src1, src2;
+ std::stop_token tok = src1.get_token();
+ VERIFY(tok.stop_possible());
+ VERIFY(!tok.stop_requested());
+
+ std::stop_token copy = src2.get_token();
+ VERIFY(copy != tok);
+ copy = tok;
+ VERIFY(copy == tok);
+ copy = src2.get_token();
+ VERIFY(copy != tok);
+}
+
+void
+test03()
{
// create stop_source
std::stop_source ssrc;
@@ -77,8 +146,6 @@ int main()
b = ssrc.request_stop();
VERIFY(!b);
- // TODO verify the standard requires this
-#if 0
// register another callback
bool cb3called{false};
std::stop_callback scb3{stok, [&]
@@ -92,5 +159,11 @@ int main()
VERIFY(!cb1called);
VERIFY(cb2called);
VERIFY(cb3called);
-#endif
+}
+
+int main()
+{
+ test01();
+ test02();
+ test03();
}
