On 15/11/19 23:43 +0000, Jonathan Wakely wrote:
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.
And a couple more bugs fixed by this patch.
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 <thread> header. We don't need
to include <functional> for std::jthread because <bits/invoke.h> is
already included, which defines std::__invoke. We can also remove
<bits/functexcept.h> 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 <thread>
anyway.
Tested powerpc64le-linux, committed to trunk.
commit f0c4ca8e20d9aadd8891bec246aecfc54177163c
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Mon Nov 18 11:45:44 2019 +0000
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.
* 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.
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 <bits/c++0x_warning.h>
#else
-#include <chrono>
-#include <memory>
-#include <tuple>
-#include <cerrno>
-
-#if __cplusplus > 201703L
-#define __cpp_lib_jthread 201907L
-#include <functional>
-#include <stop_token>
-#endif
-
-#include <bits/functexcept.h>
-#include <bits/functional_hash.h>
-#include <bits/invoke.h>
-#include <bits/gthr.h>
+#include <bits/c++config.h>
#if defined(_GLIBCXX_HAS_GTHREADS)
+#include <bits/gthr.h>
+
+#include <chrono> // std::chrono::*
+#include <memory> // std::unique_ptr
+#include <tuple> // std::tuple
+
+#if __cplusplus > 201703L
+# include <stop_token> // std::stop_source, std::stop_token, std::nostopstate
+#endif
+
+#ifdef _GLIBCXX_USE_NANOSLEEP
+# include <cerrno> // errno, EINTR
+# include <time.h> // nanosleep
+#endif
+
+#include <bits/functional_hash.h> // std::hash
+#include <bits/invoke.h> // 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<decay_t<_Callable>, 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<_Callable>,
+ 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 <thread>
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 <version>
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
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a -pthread" }
+// { dg-do compile { target c++2a } }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+#include <thread>
+
+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
// <http://www.gnu.org/licenses/>.
-// { 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 <thread>
#include <chrono>
-#include <cassert>
#include <atomic>
+#include <testsuite_hooks.h>
-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<bool> 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<bool> 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<bool> 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();
}
-
