libstdc++: Encapsulate __gthread_cond_t as std::__condvar
authorJonathan Wakely <jwakely@redhat.com>
Wed, 25 Nov 2020 14:24:21 +0000 (14:24 +0000)
committerJonathan Wakely <jwakely@redhat.com>
Wed, 25 Nov 2020 18:24:13 +0000 (18:24 +0000)
This introduces a new internal utility, std::__condvar, which is a
simplified form of std::condition_variable. It has no dependency on
<chrono> or std::unique_lock, which allows it to be used in
<bits/atomic_wait.h>.

This avoids repeating the #ifdef __GTHREAD_COND_INIT preprocessor
conditions and associated logic for initializing a __gthread_cond_t
correctly. It also encapsulates most of the __gthread_cond_xxx functions
as member functions of __condvar.

libstdc++-v3/ChangeLog:

* include/bits/atomic_timed_wait.h (__cond_wait_until_impl):
Do not define when _GLIBCXX_HAVE_LINUX_FUTEX is defined. Use
__condvar and mutex instead of __gthread_cond_t and
unique_lock<mutex>.
(__cond_wait_until): Likewise. Fix test for return value of
__cond_wait_until_impl.
(__timed_waiters::_M_do_wait_until): Use __condvar instead
of __gthread_cond_t.
* include/bits/atomic_wait.h: Remove <bits/unique_lock.h>
include. Only include <bits/std_mutex.h> if not using futexes.
(__platform_wait_max_value): Remove unused variable.
(__waiters::lock_t): Use lock_guard instead of unique_lock.
(__waiters::_M_cv): Use __condvar instead of __gthread_cond_t.
(__waiters::_M_do_wait(__platform_wait_t)): Likewise.
(__waiters::_M_notify()): Likewise. Use notify_one() if not
asked to notify all.
* include/bits/std_mutex.h (__condvar): New type.
* include/std/condition_variable (condition_variable::_M_cond)
(condition_variable::wait_until): Use __condvar instead of
__gthread_cond_t.
* src/c++11/condition_variable.cc (condition_variable): Define
default constructor and destructor as defaulted.
(condition_variable::wait, condition_variable::notify_one)
(condition_variable::notify_all): Forward to corresponding
member function of __condvar.

libstdc++-v3/include/bits/atomic_timed_wait.h
libstdc++-v3/include/bits/atomic_wait.h
libstdc++-v3/include/bits/std_mutex.h
libstdc++-v3/include/std/condition_variable
libstdc++-v3/src/c++11/condition_variable.cc

index b13f8aa1286198355cffe4b66f30ce26e1707f9a..9e44114dd5b6f5200fa5ac381a136230e5d70720 100644 (file)
@@ -40,6 +40,7 @@
 #include <chrono>
 
 #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
+#include <exception> // std::terminate
 #include <sys/time.h>
 #endif
 
@@ -113,13 +114,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            return __atomic_wait_status::timeout;
          }
       }
-#endif
+#else // ! FUTEX
 
 #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
     template<typename _Duration>
       __atomic_wait_status
-      __cond_wait_until_impl(__gthread_cond_t* __cv,
-         unique_lock<mutex>& __lock,
+      __cond_wait_until_impl(__condvar& __cv, mutex& __mx,
          const chrono::time_point<chrono::steady_clock, _Duration>& __atime)
       {
        auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
@@ -131,62 +131,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            static_cast<long>(__ns.count())
          };
 
-       pthread_cond_clockwait(__cv, __lock.mutex()->native_handle(),
-                              CLOCK_MONOTONIC,
-                              &__ts);
+       __cv.wait_until(__mx, CLOCK_MONOTONIC, __ts);
+
        return (chrono::steady_clock::now() < __atime)
               ? __atomic_wait_status::no_timeout
               : __atomic_wait_status::timeout;
       }
 #endif
 
-      template<typename _Duration>
-       __atomic_wait_status
-       __cond_wait_until_impl(__gthread_cond_t* __cv,
-           unique_lock<std::mutex>& __lock,
-           const chrono::time_point<chrono::system_clock, _Duration>& __atime)
+    template<typename _Duration>
+      __atomic_wait_status
+      __cond_wait_until_impl(__condvar& __cv, mutex& __mx,
+         const chrono::time_point<chrono::system_clock, _Duration>& __atime)
+      {
+       auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
+       auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
+
+       __gthread_time_t __ts =
        {
-         auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
-         auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
+         static_cast<std::time_t>(__s.time_since_epoch().count()),
+         static_cast<long>(__ns.count())
+       };
 
-         __gthread_time_t __ts =
-         {
-           static_cast<std::time_t>(__s.time_since_epoch().count()),
-           static_cast<long>(__ns.count())
-         };
+       __cv.wait_until(__mx, __ts);
 
-         __gthread_cond_timedwait(__cv, __lock.mutex()->native_handle(),
-                                  &__ts);
-         return (chrono::system_clock::now() < __atime)
-                ? __atomic_wait_status::no_timeout
-                : __atomic_wait_status::timeout;
-       }
+       return (chrono::system_clock::now() < __atime)
+              ? __atomic_wait_status::no_timeout
+              : __atomic_wait_status::timeout;
+      }
 
-      // return true if timeout
-      template<typename _Clock, typename _Duration>
-       __atomic_wait_status
-       __cond_wait_until(__gthread_cond_t* __cv,
-           unique_lock<std::mutex>& __lock,
-           const chrono::time_point<_Clock, _Duration>& __atime)
-       {
+    // return true if timeout
+    template<typename _Clock, typename _Duration>
+      __atomic_wait_status
+      __cond_wait_until(__condvar& __cv, mutex& __mx,
+         const chrono::time_point<_Clock, _Duration>& __atime)
+      {
 #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
-         using __clock_t = chrono::steady_clock;
+       using __clock_t = chrono::steady_clock;
 #else
-         using __clock_t = chrono::system_clock;
+       using __clock_t = chrono::system_clock;
 #endif
-         const typename _Clock::time_point __c_entry = _Clock::now();
-         const __clock_t::time_point __s_entry = __clock_t::now();
-         const auto __delta = __atime - __c_entry;
-         const auto __s_atime = __s_entry + __delta;
-         if (std::__detail::__cond_wait_until_impl(__cv, __lock, __s_atime))
-           return __atomic_wait_status::no_timeout;
-         // We got a timeout when measured against __clock_t but
-         // we need to check against the caller-supplied clock
-         // to tell whether we should return a timeout.
-         if (_Clock::now() < __atime)
-           return __atomic_wait_status::no_timeout;
-         return __atomic_wait_status::timeout;
-       }
+       const typename _Clock::time_point __c_entry = _Clock::now();
+       const __clock_t::time_point __s_entry = __clock_t::now();
+       const auto __delta = __atime - __c_entry;
+       const auto __s_atime = __s_entry + __delta;
+       if (__detail::__cond_wait_until_impl(__cv, __mx, __s_atime)
+           == __atomic_wait_status::no_timeout)
+         return __atomic_wait_status::no_timeout;
+       // We got a timeout when measured against __clock_t but
+       // we need to check against the caller-supplied clock
+       // to tell whether we should return a timeout.
+       if (_Clock::now() < __atime)
+         return __atomic_wait_status::no_timeout;
+       return __atomic_wait_status::timeout;
+      }
+#endif // FUTEX
 
     struct __timed_waiters : __waiters
     {
@@ -202,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
          __waiters::__lock_t __l(_M_mtx);
          while (__cur <= __version)
            {
-             if (__detail::__cond_wait_until(&_M_cv, __l, __atime)
+             if (__detail::__cond_wait_until(_M_cv, _M_mtx, __atime)
                    == __atomic_wait_status::timeout)
                return __atomic_wait_status::timeout;
 
@@ -281,7 +280,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       if (__reltime < __rtime)
        ++__reltime;
 
-
       return __atomic_wait_until(__addr, __old, std::move(__pred),
                                 chrono::steady_clock::now() + __reltime);
     }
index 5af9367ca2e9a94a52324504e600722bb2c7db10..3aaaa9aaec35b9fa7845547f43d7b7b9072cb997 100644 (file)
 #if defined _GLIBCXX_HAS_GTHREADS || _GLIBCXX_HAVE_LINUX_FUTEX
 #include <bits/functional_hash.h>
 #include <bits/gthr.h>
-#include <bits/std_mutex.h>
-#include <bits/unique_lock.h>
 #include <ext/numeric_traits.h>
 
 #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
-#include <climits>
-#include <unistd.h>
-#include <syscall.h>
+# include <cerrno>
+# include <climits>
+# include <unistd.h>
+# include <syscall.h>
+# include <bits/functexcept.h>
+// TODO get this from Autoconf
+# define _GLIBCXX_HAVE_LINUX_FUTEX_PRIVATE 1
+#else
+# include <bits/std_mutex.h>  // std::mutex, std::__condvar
 #endif
 
 
-// TODO get this from Autoconf
-#define _GLIBCXX_HAVE_LINUX_FUTEX_PRIVATE 1
-
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -60,10 +61,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     constexpr auto __atomic_spin_count_1 = 16;
     constexpr auto __atomic_spin_count_2 = 12;
 
-    inline constexpr
-    auto __platform_wait_max_value =
-               __gnu_cxx::__int_traits<__platform_wait_t>::__max;
-
     template<typename _Tp>
       inline constexpr bool __platform_wait_uses_type
 #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
@@ -119,23 +116,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     struct __waiters
     {
-       alignas(64) __platform_wait_t _M_ver = 0;
-       alignas(64) __platform_wait_t _M_wait = 0;
+      alignas(64) __platform_wait_t _M_ver = 0;
+      alignas(64) __platform_wait_t _M_wait = 0;
 
 #ifndef _GLIBCXX_HAVE_LINUX_FUTEX
-      using __lock_t = std::unique_lock<std::mutex>;
-      mutable __lock_t::mutex_type _M_mtx;
+      using __lock_t = lock_guard<mutex>;
+      mutex _M_mtx;
+      __condvar _M_cv;
 
-#  ifdef __GTHREAD_COND_INIT
-      mutable __gthread_cond_t _M_cv = __GTHREAD_COND_INIT;
       __waiters() noexcept = default;
-#  else
-      mutable __gthread_cond_t _M_cv;
-      __waiters() noexcept
-      {
-       __GTHREAD_COND_INIT_FUNCTION(&_M_cv);
-      }
-#  endif
 #endif
 
       __platform_wait_t
@@ -163,9 +152,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        while (__cur <= __version)
          {
            __waiters::__lock_t __l(_M_mtx);
-           auto __e = __gthread_cond_wait(&_M_cv, __l.mutex()->native_handle());
-           if (__e)
-             __throw_system_error(__e);
+           _M_cv.wait(_M_mtx);
            __platform_wait_t __last = __cur;
            __atomic_load(&_M_ver, &__cur, __ATOMIC_ACQUIRE);
            if (__cur < __last)
@@ -189,9 +176,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
        __platform_notify(&_M_ver, __all);
 #else
-       auto __e = __gthread_cond_broadcast(&_M_cv);
-       if (__e)
-         __throw_system_error(__e);
+       if (__all)
+         _M_cv.notify_all();
+       else
+         _M_cv.notify_one();
 #endif
       }
 
index 56c853a3fdcefd4fbb7974dbe843169c162f168a..f308bf35437ebe309efde5344b2384baf553b09b 100644 (file)
@@ -123,6 +123,76 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return &_M_mutex; }
   };
 
+  // Implementation details for std::condition_variable
+  class __condvar
+  {
+    using timespec = __gthread_time_t;
+
+  public:
+    __condvar() noexcept
+    {
+#ifndef __GTHREAD_COND_INIT
+      __GTHREAD_COND_INIT_FUNCTION(&_M_cond);
+#endif
+    }
+
+    ~__condvar()
+    {
+      int __e __attribute__((__unused__)) = __gthread_cond_destroy(&_M_cond);
+      __glibcxx_assert(__e != EBUSY); // threads are still blocked
+    }
+
+    __condvar(const __condvar&) = delete;
+    __condvar& operator=(const __condvar&) = delete;
+
+    __gthread_cond_t* native_handle() noexcept { return &_M_cond; }
+
+    // Expects: Calling thread has locked __m.
+    void
+    wait(mutex& __m) noexcept
+    {
+      int __e __attribute__((__unused__))
+       = __gthread_cond_wait(&_M_cond, __m.native_handle());
+      __glibcxx_assert(__e == 0);
+    }
+
+    void
+    wait_until(mutex& __m, timespec& __abs_time) noexcept
+    {
+      __gthread_cond_timedwait(&_M_cond, __m.native_handle(), &__abs_time);
+    }
+
+#ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
+    void
+    wait_until(mutex& __m, clockid_t __clock, timespec& __abs_time) noexcept
+    {
+      pthread_cond_clockwait(&_M_cond, __m.native_handle(), __clock,
+                            &__abs_time);
+    }
+#endif
+
+    void
+    notify_one() noexcept
+    {
+      int __e __attribute__((__unused__)) = __gthread_cond_signal(&_M_cond);
+      __glibcxx_assert(__e == 0);
+    }
+
+    void
+    notify_all() noexcept
+    {
+      int __e __attribute__((__unused__)) = __gthread_cond_broadcast(&_M_cond);
+      __glibcxx_assert(__e == 0);
+    }
+
+  protected:
+#ifdef __GTHREAD_COND_INIT
+    __gthread_cond_t _M_cond = __GTHREAD_COND_INIT;
+#else
+    __gthread_cond_t _M_cond;
+#endif
+  };
+
 #endif // _GLIBCXX_HAS_GTHREADS
 
   /// Do not acquire ownership of the mutex.
index 7406fde6e4b40c8f8bf9e7c4d6dc4a99a8c941df..5b7a9cb35a8636db38357572459363e5bb18f240 100644 (file)
@@ -74,16 +74,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #else
     using __clock_t = system_clock;
 #endif
-    typedef __gthread_cond_t           __native_type;
 
-#ifdef __GTHREAD_COND_INIT
-    __native_type                      _M_cond = __GTHREAD_COND_INIT;
-#else
-    __native_type                      _M_cond;
-#endif
+    __condvar _M_cond;
 
   public:
-    typedef __native_type*             native_handle_type;
+    typedef __gthread_cond_t*          native_handle_type;
 
     condition_variable() noexcept;
     ~condition_variable() noexcept;
@@ -185,7 +180,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     native_handle_type
     native_handle()
-    { return &_M_cond; }
+    { return _M_cond.native_handle(); }
 
   private:
 #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
@@ -203,9 +198,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            static_cast<long>(__ns.count())
          };
 
-       pthread_cond_clockwait(&_M_cond, __lock.mutex()->native_handle(),
-                                        CLOCK_MONOTONIC,
-                                        &__ts);
+       _M_cond.wait_until(*__lock.mutex(), CLOCK_MONOTONIC, __ts);
 
        return (steady_clock::now() < __atime
                ? cv_status::no_timeout : cv_status::timeout);
@@ -226,8 +219,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            static_cast<long>(__ns.count())
          };
 
-       __gthread_cond_timedwait(&_M_cond, __lock.mutex()->native_handle(),
-                                &__ts);
+       _M_cond.wait_until(*__lock.mutex(), __ts);
 
        return (system_clock::now() < __atime
                ? cv_status::no_timeout : cv_status::timeout);
index 19b03da8e205c0237f1260be8577c985ff09928e..92721bba3a97962c3343b10d36340341fe8be232 100644 (file)
@@ -31,51 +31,26 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-#ifdef __GTHREAD_COND_INIT
   condition_variable::condition_variable() noexcept = default;
-#else
-  condition_variable::condition_variable() noexcept
-  {
-    __GTHREAD_COND_INIT_FUNCTION(&_M_cond);
-  }
-#endif
 
-  condition_variable::~condition_variable() noexcept
-  {
-    // XXX no thread blocked
-    /* int __e = */ __gthread_cond_destroy(&_M_cond);
-    // if __e == EBUSY then blocked
-  }
+  condition_variable::~condition_variable() noexcept = default;
 
   void
   condition_variable::wait(unique_lock<mutex>& __lock) noexcept
   {
-    int __e = __gthread_cond_wait(&_M_cond, __lock.mutex()->native_handle());
-
-    if (__e)
-      std::terminate();
+    _M_cond.wait(*__lock.mutex());
   }
 
   void
   condition_variable::notify_one() noexcept
   {
-    int __e = __gthread_cond_signal(&_M_cond);
-
-    // XXX not in spec
-    // EINVAL
-    if (__e)
-      __throw_system_error(__e);
+    _M_cond.notify_one();
   }
 
   void
   condition_variable::notify_all() noexcept
   {
-    int __e = __gthread_cond_broadcast(&_M_cond);
-
-    // XXX not in spec
-    // EINVAL
-    if (__e)
-      __throw_system_error(__e);
+    _M_cond.notify_all();
   }
 
   extern void