From ebc464940de77b8e22f99d1e2b1a1a3bf23eb24d Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Mon, 18 Nov 2019 12:46:08 +0000 Subject: [PATCH] libstdc++: Fix std::jthread bugs The std::jthread::get_id() function was missing a return statement. The is_invocable check needs to be done using decayed types, as they'll be forwarded to std::invoke as rvalues. Also reduce header dependencies for the header. We don't need to include for std::jthread because is already included, which defines std::__invoke. We can also remove which isn't used at all. Finally, when _GLIBCXX_HAS_GTHREADS is not defined there's no point including any other headers, since we're not going to define anything in anyway. * include/std/thread: Reduce header dependencies. (jthread::get_id()): Add missing return. (jthread::get_stop_token()): Avoid unnecessary stop_source temporary. (jthread::_S_create): Check is_invocable using decayed types. Add static assertion. * testsuite/30_threads/jthread/1.cc: Add dg-require-gthreads. * testsuite/30_threads/jthread/2.cc: Likewise. * testsuite/30_threads/jthread/3.cc: New test. * testsuite/30_threads/jthread/jthread.cc: Add missing directives for pthread and gthread support. Use VERIFY instead of assert. From-SVN: r278402 --- libstdc++-v3/ChangeLog | 11 +++ libstdc++-v3/include/std/thread | 47 +++++----- .../testsuite/30_threads/jthread/1.cc | 1 + .../testsuite/30_threads/jthread/2.cc | 1 + .../testsuite/30_threads/jthread/3.cc | 45 ++++++++++ .../testsuite/30_threads/jthread/jthread.cc | 89 ++++++++++--------- 6 files changed, 130 insertions(+), 64 deletions(-) create mode 100644 libstdc++-v3/testsuite/30_threads/jthread/3.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index d2162ae28bb..6f493120ef4 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,16 @@ 2019-11-18 Jonathan Wakely + * include/std/thread: Reduce header dependencies. + (jthread::get_id()): Add missing return. + (jthread::get_stop_token()): Avoid unnecessary stop_source temporary. + (jthread::_S_create): Check is_invocable using decayed types. Add + static assertion. + * testsuite/30_threads/jthread/1.cc: Add dg-require-gthreads. + * testsuite/30_threads/jthread/2.cc: Likewise. + * testsuite/30_threads/jthread/3.cc: New test. + * testsuite/30_threads/jthread/jthread.cc: Add missing directives for + pthread and gthread support. Use VERIFY instead of assert. + * include/bits/alloc_traits.h (allocator_traits::construct) (allocator_traits::destroy, allocator_traits::max_size): Add unused attributes to parameters that are not used in C++20. diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread index 010921b2160..825ce4096fc 100644 --- a/libstdc++-v3/include/std/thread +++ b/libstdc++-v3/include/std/thread @@ -35,23 +35,26 @@ # include #else -#include -#include -#include -#include +#include + +#if defined(_GLIBCXX_HAS_GTHREADS) +#include + +#include // std::chrono::* +#include // std::unique_ptr +#include // std::tuple #if __cplusplus > 201703L -#define __cpp_lib_jthread 201907L -#include -#include +# include // std::stop_source, std::stop_token, std::nostopstate #endif -#include -#include -#include -#include +#ifdef _GLIBCXX_USE_NANOSLEEP +# include // errno, EINTR +# include // nanosleep +#endif -#if defined(_GLIBCXX_HAS_GTHREADS) +#include // std::hash +#include // std::__invoke namespace std _GLIBCXX_VISIBILITY(default) { @@ -483,7 +486,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION [[nodiscard]] id get_id() const noexcept { - _M_thread.get_id(); + return _M_thread.get_id(); } [[nodiscard]] native_handle_type @@ -512,7 +515,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION bool request_stop() noexcept { - return get_stop_source().request_stop(); + return _M_stop_source.request_stop(); } friend void swap(jthread& __lhs, jthread& __rhs) noexcept @@ -525,12 +528,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static thread _S_create(stop_source& __ssrc, _Callable&& __f, _Args&&... __args) { - if constexpr(is_invocable_v<_Callable, stop_token, _Args...>) + if constexpr(is_invocable_v, stop_token, + decay_t<_Args>...>) return thread{std::forward<_Callable>(__f), __ssrc.get_token(), std::forward<_Args>(__args)...}; else - return thread{std::forward<_Callable>(__f), - std::forward<_Args>(__args)...}; + { + static_assert(is_invocable_v, + decay_t<_Args>...>, + "std::thread arguments must be invocable after" + " conversion to rvalues"); + return thread{std::forward<_Callable>(__f), + std::forward<_Args>(__args)...}; + } } stop_source _M_stop_source; @@ -539,9 +549,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #endif // __cpp_lib_jthread _GLIBCXX_END_NAMESPACE_VERSION } // namespace - #endif // _GLIBCXX_HAS_GTHREADS - #endif // C++11 - #endif // _GLIBCXX_THREAD diff --git a/libstdc++-v3/testsuite/30_threads/jthread/1.cc b/libstdc++-v3/testsuite/30_threads/jthread/1.cc index 1fb5650dbc6..574c1f25dc3 100644 --- a/libstdc++-v3/testsuite/30_threads/jthread/1.cc +++ b/libstdc++-v3/testsuite/30_threads/jthread/1.cc @@ -17,6 +17,7 @@ // { dg-options "-std=gnu++2a" } // { dg-do compile { target c++2a } } +// { dg-require-gthreads "" } #include diff --git a/libstdc++-v3/testsuite/30_threads/jthread/2.cc b/libstdc++-v3/testsuite/30_threads/jthread/2.cc index 621965c8910..52413c8d2a3 100644 --- a/libstdc++-v3/testsuite/30_threads/jthread/2.cc +++ b/libstdc++-v3/testsuite/30_threads/jthread/2.cc @@ -17,6 +17,7 @@ // { dg-options "-std=gnu++2a" } // { dg-do compile { target c++2a } } +// { dg-require-gthreads "" } #include diff --git a/libstdc++-v3/testsuite/30_threads/jthread/3.cc b/libstdc++-v3/testsuite/30_threads/jthread/3.cc new file mode 100644 index 00000000000..e4a2fbb3620 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/jthread/3.cc @@ -0,0 +1,45 @@ +// 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 +// . + +// { dg-options "-std=gnu++2a -pthread" } +// { dg-do compile { target c++2a } } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } + +#include + +struct RvalCallable +{ + void operator()() && { } +}; + +void test01() +{ + RvalCallable r; + std::jthread t(r); +} + +struct RvalCallableWithToken +{ + void operator()(std::stop_token) && { } +}; + +void test02() +{ + RvalCallableWithToken r; + std::jthread t(r); +} diff --git a/libstdc++-v3/testsuite/30_threads/jthread/jthread.cc b/libstdc++-v3/testsuite/30_threads/jthread/jthread.cc index c29db212167..aeb9f1f1b7f 100644 --- a/libstdc++-v3/testsuite/30_threads/jthread/jthread.cc +++ b/libstdc++-v3/testsuite/30_threads/jthread/jthread.cc @@ -15,15 +15,17 @@ // with this library; see the file COPYING3. If not see // . -// { dg-options "-std=gnu++2a" } -// { dg-do compile { target c++2a } } +// { dg-options "-std=gnu++2a -pthread" } +// { dg-do run { target c++2a } } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } #include #include -#include #include +#include -using namespace::std::literals; +using namespace std::literals; //------------------------------------------------------ @@ -31,9 +33,9 @@ void test_no_stop_token() { // test the basic jthread API (not taking stop_token arg) - assert(std::jthread::hardware_concurrency() == std::thread::hardware_concurrency()); + VERIFY(std::jthread::hardware_concurrency() == std::thread::hardware_concurrency()); std::stop_token stoken; - assert(!stoken.stop_possible()); + VERIFY(!stoken.stop_possible()); { std::jthread::id t1ID{std::this_thread::get_id()}; std::atomic t1AllSet{false}; @@ -47,12 +49,12 @@ void test_no_stop_token() for (int i=0; !t1AllSet.load(); ++i) { std::this_thread::sleep_for(10ms); } - assert(t1.joinable()); - assert(t1ID == t1.get_id()); + VERIFY(t1.joinable()); + VERIFY(t1ID == t1.get_id()); stoken = t1.get_stop_token(); - assert(!stoken.stop_requested()); - } - assert(stoken.stop_requested()); + VERIFY(!stoken.stop_requested()); + } + VERIFY(stoken.stop_requested()); } //------------------------------------------------------ @@ -63,8 +65,8 @@ void test_stop_token() std::stop_source ssource; std::stop_source origsource; - assert(ssource.stop_possible()); - assert(!ssource.stop_requested()); + VERIFY(ssource.stop_possible()); + VERIFY(!ssource.stop_requested()); { std::jthread::id t1ID{std::this_thread::get_id()}; std::atomic t1AllSet{false}; @@ -83,26 +85,26 @@ void test_stop_token() std::this_thread::sleep_for(10ms); } // and check all values: - assert(t1.joinable()); - assert(t1ID == t1.get_id()); + VERIFY(t1.joinable()); + VERIFY(t1ID == t1.get_id()); std::this_thread::sleep_for(470ms); origsource = std::move(ssource); ssource = t1.get_stop_source(); - assert(!ssource.stop_requested()); + VERIFY(!ssource.stop_requested()); auto ret = ssource.request_stop(); - assert(ret); + VERIFY(ret); ret = ssource.request_stop(); - assert(!ret); - assert(ssource.stop_requested()); - assert(!t1done.load()); - assert(!origsource.stop_requested()); + VERIFY(!ret); + VERIFY(ssource.stop_requested()); + VERIFY(!t1done.load()); + VERIFY(!origsource.stop_requested()); std::this_thread::sleep_for(470ms); origsource.request_stop(); - } - assert(origsource.stop_requested()); - assert(ssource.stop_requested()); + } + VERIFY(origsource.stop_requested()); + VERIFY(ssource.stop_requested()); } //------------------------------------------------------ @@ -110,7 +112,7 @@ void test_stop_token() void test_join() { std::stop_source ssource; - assert(ssource.stop_possible()); + VERIFY(ssource.stop_possible()); { std::jthread t1([](std::stop_token stoken) { for (int i=0; !stoken.stop_requested(); ++i) { @@ -126,10 +128,10 @@ void test_join() }); // wait for all thread to finish: t2.join(); - assert(!t2.joinable()); - assert(t1.joinable()); + VERIFY(!t2.joinable()); + VERIFY(t1.joinable()); t1.join(); - assert(!t1.joinable()); + VERIFY(!t1.joinable()); } } @@ -138,7 +140,7 @@ void test_join() void test_detach() { std::stop_source ssource; - assert(ssource.stop_possible()); + VERIFY(ssource.stop_possible()); std::atomic t1FinallyInterrupted{false}; { std::jthread t0; @@ -152,8 +154,8 @@ void test_detach() t1ID = std::this_thread::get_id(); t1InterruptToken = stoken; t1IsInterrupted = stoken.stop_requested(); - assert(stoken.stop_possible()); - assert(!stoken.stop_requested()); + VERIFY(stoken.stop_possible()); + VERIFY(!stoken.stop_requested()); t1AllSet.store(true); for (int i=0; !stoken.stop_requested(); ++i) { std::this_thread::sleep_for(100ms); @@ -163,31 +165,31 @@ void test_detach() for (int i=0; !t1AllSet.load(); ++i) { std::this_thread::sleep_for(10ms); } - assert(!t0.joinable()); - assert(t1.joinable()); - assert(t1ID == t1.get_id()); - assert(t1IsInterrupted == false); - assert(t1InterruptToken == t1.get_stop_source().get_token()); + VERIFY(!t0.joinable()); + VERIFY(t1.joinable()); + VERIFY(t1ID == t1.get_id()); + VERIFY(t1IsInterrupted == false); + VERIFY(t1InterruptToken == t1.get_stop_source().get_token()); ssource = t1.get_stop_source(); - assert(t1InterruptToken.stop_possible()); - assert(!t1InterruptToken.stop_requested()); + VERIFY(t1InterruptToken.stop_possible()); + VERIFY(!t1InterruptToken.stop_requested()); t1.detach(); - assert(!t1.joinable()); + VERIFY(!t1.joinable()); } - assert(!t1FinallyInterrupted.load()); + VERIFY(!t1FinallyInterrupted.load()); ssource.request_stop(); - assert(ssource.stop_requested()); + VERIFY(ssource.stop_requested()); for (int i=0; !t1FinallyInterrupted.load() && i < 100; ++i) { std::this_thread::sleep_for(100ms); } - assert(t1FinallyInterrupted.load()); + VERIFY(t1FinallyInterrupted.load()); } int main() { std::set_terminate([](){ - assert(false); + VERIFY(false); }); test_no_stop_token(); @@ -195,4 +197,3 @@ int main() test_join(); test_detach(); } - -- 2.30.2