libstdc++: Fix std::jthread bugs
authorJonathan Wakely <jwakely@redhat.com>
Mon, 18 Nov 2019 12:46:08 +0000 (12:46 +0000)
committerJonathan Wakely <redi@gcc.gnu.org>
Mon, 18 Nov 2019 12:46:08 +0000 (12:46 +0000)
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

libstdc++-v3/ChangeLog
libstdc++-v3/include/std/thread
libstdc++-v3/testsuite/30_threads/jthread/1.cc
libstdc++-v3/testsuite/30_threads/jthread/2.cc
libstdc++-v3/testsuite/30_threads/jthread/3.cc [new file with mode: 0644]
libstdc++-v3/testsuite/30_threads/jthread/jthread.cc

index d2162ae28bb357ddecf907a9b77abcc2da705a76..6f493120ef4f4622d72467fd18c03df5ecfd4424 100644 (file)
@@ -1,5 +1,16 @@
 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.
index 010921b2160f180c348388bf049341bec522376b..825ce4096fcfd0de4b3ba3a94b2cba417f4ad15d 100644 (file)
 # 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
index 1fb5650dbc6d83f0c6b9ad398623b9fd91d8555b..574c1f25dc3c694fca838cd82ee72b192c9b5a9f 100644 (file)
@@ -17,6 +17,7 @@
 
 // { dg-options "-std=gnu++2a" }
 // { dg-do compile { target c++2a } }
+// { dg-require-gthreads "" }
 
 #include <thread>
 
index 621965c89102ff0d3a15a3c39522c254e20396e9..52413c8d2a3e6c1625575a52dfa0d04772d34bd3 100644 (file)
@@ -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 (file)
index 0000000..e4a2fbb
--- /dev/null
@@ -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);
+}
index c29db21216789f89b3f4b0bb1efb3e44e69949c6..aeb9f1f1b7f8c7da7886efbf1feb5421a6fd01bd 100644 (file)
 // 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();
 }
-