Commit ebc46494 by Jonathan Wakely Committed by Jonathan Wakely

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 <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.

	* 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
parent 3b39526e
2019-11-18 Jonathan Wakely <jwakely@redhat.com>
* 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.
......
......@@ -35,23 +35,26 @@
# include <bits/c++0x_warning.h>
#else
#include <chrono>
#include <memory>
#include <tuple>
#include <cerrno>
#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
#define __cpp_lib_jthread 201907L
#include <functional>
#include <stop_token>
# include <stop_token> // std::stop_source, std::stop_token, std::nostopstate
#endif
#include <bits/functexcept.h>
#include <bits/functional_hash.h>
#include <bits/invoke.h>
#include <bits/gthr.h>
#ifdef _GLIBCXX_USE_NANOSLEEP
# include <cerrno> // errno, EINTR
# include <time.h> // nanosleep
#endif
#if defined(_GLIBCXX_HAS_GTHREADS)
#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
......@@ -17,6 +17,7 @@
// { dg-options "-std=gnu++2a" }
// { dg-do compile { target c++2a } }
// { dg-require-gthreads "" }
#include <thread>
......
......@@ -17,6 +17,7 @@
// { dg-options "-std=gnu++2a" }
// { dg-do compile { target c++2a } }
// { dg-require-gthreads "" }
#include <version>
......
// 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);
}
......@@ -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();
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment