From 7d2a98a7273c423842a3935de64b15a6d6cb33bc Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 25 Nov 2020 14:24:21 +0000 Subject: [PATCH] libstdc++: Encapsulate __gthread_cond_t as std::__condvar This introduces a new internal utility, std::__condvar, which is a simplified form of std::condition_variable. It has no dependency on or std::unique_lock, which allows it to be used in . 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. (__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 include. Only include 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 | 94 +++++++++---------- libstdc++-v3/include/bits/atomic_wait.h | 50 ++++------ libstdc++-v3/include/bits/std_mutex.h | 70 ++++++++++++++ libstdc++-v3/include/std/condition_variable | 18 +--- libstdc++-v3/src/c++11/condition_variable.cc | 33 +------ 5 files changed, 144 insertions(+), 121 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h b/libstdc++-v3/include/bits/atomic_timed_wait.h index b13f8aa1286..9e44114dd5b 100644 --- a/libstdc++-v3/include/bits/atomic_timed_wait.h +++ b/libstdc++-v3/include/bits/atomic_timed_wait.h @@ -40,6 +40,7 @@ #include #ifdef _GLIBCXX_HAVE_LINUX_FUTEX +#include // std::terminate #include #endif @@ -113,13 +114,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return __atomic_wait_status::timeout; } } -#endif +#else // ! FUTEX #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT template __atomic_wait_status - __cond_wait_until_impl(__gthread_cond_t* __cv, - unique_lock& __lock, + __cond_wait_until_impl(__condvar& __cv, mutex& __mx, const chrono::time_point& __atime) { auto __s = chrono::time_point_cast(__atime); @@ -131,62 +131,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static_cast(__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 - __atomic_wait_status - __cond_wait_until_impl(__gthread_cond_t* __cv, - unique_lock& __lock, - const chrono::time_point& __atime) + template + __atomic_wait_status + __cond_wait_until_impl(__condvar& __cv, mutex& __mx, + const chrono::time_point& __atime) + { + auto __s = chrono::time_point_cast(__atime); + auto __ns = chrono::duration_cast(__atime - __s); + + __gthread_time_t __ts = { - auto __s = chrono::time_point_cast(__atime); - auto __ns = chrono::duration_cast(__atime - __s); + static_cast(__s.time_since_epoch().count()), + static_cast(__ns.count()) + }; - __gthread_time_t __ts = - { - static_cast(__s.time_since_epoch().count()), - static_cast(__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 - __atomic_wait_status - __cond_wait_until(__gthread_cond_t* __cv, - unique_lock& __lock, - const chrono::time_point<_Clock, _Duration>& __atime) - { + // return true if timeout + template + __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); } diff --git a/libstdc++-v3/include/bits/atomic_wait.h b/libstdc++-v3/include/bits/atomic_wait.h index 5af9367ca2e..3aaaa9aaec3 100644 --- a/libstdc++-v3/include/bits/atomic_wait.h +++ b/libstdc++-v3/include/bits/atomic_wait.h @@ -36,20 +36,21 @@ #if defined _GLIBCXX_HAS_GTHREADS || _GLIBCXX_HAVE_LINUX_FUTEX #include #include -#include -#include #include #ifdef _GLIBCXX_HAVE_LINUX_FUTEX -#include -#include -#include +# include +# include +# include +# include +# include +// TODO get this from Autoconf +# define _GLIBCXX_HAVE_LINUX_FUTEX_PRIVATE 1 +#else +# include // 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 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; - mutable __lock_t::mutex_type _M_mtx; + using __lock_t = lock_guard; + 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 } diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h index 56c853a3fdc..f308bf35437 100644 --- a/libstdc++-v3/include/bits/std_mutex.h +++ b/libstdc++-v3/include/bits/std_mutex.h @@ -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. diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable index 7406fde6e4b..5b7a9cb35a8 100644 --- a/libstdc++-v3/include/std/condition_variable +++ b/libstdc++-v3/include/std/condition_variable @@ -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(__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(__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); diff --git a/libstdc++-v3/src/c++11/condition_variable.cc b/libstdc++-v3/src/c++11/condition_variable.cc index 19b03da8e20..92721bba3a9 100644 --- a/libstdc++-v3/src/c++11/condition_variable.cc +++ b/libstdc++-v3/src/c++11/condition_variable.cc @@ -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& __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 -- 2.30.2