If pthread_cond_timedwaitonclock_np is available in the C library it can be used it to fix part of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 by supporting std::chrono::steady_clock properly with std::condition_variable.
This means that code using std::condition_variable::wait_for or std::condition_variable::wait_until with std::chrono::steady_clock is no longer subject to timing out early or potentially waiting for much longer if the system clock is changed at an inopportune moment. If pthread_cond_timedwaitonclock_np is available then std::chrono::steady_clock is deemed to be the "best" clock available which means that it is used for the relative wait_for calls and absolute wait_until calls using user-defined clocks. Calls explicitly using std::chrono::system_clock continue to use CLOCK_REALTIME. If pthread_cond_timedwaitonclock_np is not available then std::chrono::system_clock is deemed to be the "best" clock available which means that the previous suboptimal behaviour remains. (I have a patch that adds pthread_cond_timedwaitonclock_np to glibc ready to submit but I wanted to get feedback on this part of the change before submitting the glibc patch. See https://gitlab.com/mikecrowe/glibc-monotonic .) Signed-off-by: Mike Crowe <m...@mcrowe.com> --- libstdc++-v3/acinclude.m4 | 31 ++++++++ libstdc++-v3/config.h.in | 3 + libstdc++-v3/configure | 83 ++++++++++++++++++++++ libstdc++-v3/configure.ac | 3 + libstdc++-v3/include/std/condition_variable | 57 ++++++++++++--- .../30_threads/condition_variable/members/2.cc | 27 ++++++- .../30_threads/condition_variable_any/members/2.cc | 27 ++++++- 7 files changed, 217 insertions(+), 14 deletions(-) diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index 11f48f9..c4d369d 100644 --- a/libstdc++-v3/acinclude.m4 +++ b/libstdc++-v3/acinclude.m4 @@ -3652,6 +3652,37 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREADS_NUM_PROCESSORS_NP], [ ]) dnl +dnl Check whether pthread_cond_timedwaitonclock_np is available in <pthread.h> for std::condition_variable to use, +dnl and define _GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP. +dnl +AC_DEFUN([GLIBCXX_CHECK_PTHREAD_COND_TIMEDWAITONCLOCK_NP], [ + + AC_LANG_SAVE + AC_LANG_CPLUSPLUS + ac_save_CXXFLAGS="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS -fno-exceptions" + ac_save_LIBS="$LIBS" + LIBS="$LIBS -lpthread" + + AC_MSG_CHECKING([for pthread_cond_timedwaitonclock_np]) + AC_CACHE_VAL(glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP, [ + GCC_TRY_COMPILE_OR_LINK( + [#include <pthread.h>], + [pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_timedwaitonclock_np(&cond, &mutex, 0, &ts);], + [glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=yes], + [glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=no]) + ]) + if test $glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP = yes; then + AC_DEFINE(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP, 1, [Define if pthread_cond_timedwaitonclock_np is available in <pthread.h>.]) + fi + AC_MSG_RESULT($glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP) + + CXXFLAGS="$ac_save_CXXFLAGS" + LIBS="$ac_save_LIBS" + AC_LANG_RESTORE +]) + +dnl dnl Check whether sysctl is available in <pthread.h>, and define _GLIBCXX_USE_SYSCTL_HW_NCPU. dnl AC_DEFUN([GLIBCXX_CHECK_SYSCTL_HW_NCPU], [ diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in index 337f614..a46efd8 100644 --- a/libstdc++-v3/config.h.in +++ b/libstdc++-v3/config.h.in @@ -869,6 +869,9 @@ /* Define if pthreads_num_processors_np is available in <pthread.h>. */ #undef _GLIBCXX_USE_PTHREADS_NUM_PROCESSORS_NP +/* Define if pthread_cond_timedwaitonclock_np is available in <pthread.h>. */ +#undef _GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP + /* Define if POSIX read/write locks are available in <gthr.h>. */ #undef _GLIBCXX_USE_PTHREAD_RWLOCK_T diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure index e9521d6..19434c4 100755 --- a/libstdc++-v3/configure +++ b/libstdc++-v3/configure @@ -20220,6 +20220,89 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu +# For pthread_cond_timedwaitonclock_np + + + + ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + + ac_save_CXXFLAGS="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS -fno-exceptions" + ac_save_LIBS="$LIBS" + LIBS="$LIBS -lpthread" + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pthread_cond_timedwaitonclock_np" >&5 +$as_echo_n "checking for pthread_cond_timedwaitonclock_np... " >&6; } + if test "${glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP+set}" = set; then : + $as_echo_n "(cached) " >&6 +else + + if test x$gcc_no_link = xyes; then + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <pthread.h> +int +main () +{ +pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_timedwaitonclock_np(&cond, &mutex, 0, &ts); + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=yes +else + glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +else + if test x$gcc_no_link = xyes; then + as_fn_error "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5 +fi +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <pthread.h> +int +main () +{ +pthread_mutex_t mutex; pthread_cond_t cond; struct timespec ts; int n = pthread_cond_timedwaitonclock_np(&cond, &mutex, 0, &ts); + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_link "$LINENO"; then : + glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=yes +else + glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP=no +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi + +fi + + if test $glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP = yes; then + +$as_echo "#define _GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP 1" >>confdefs.h + + fi + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP" >&5 +$as_echo "$glibcxx_cv_PTHREAD_COND_TIMEDWAITONCLOCK_NP" >&6; } + + CXXFLAGS="$ac_save_CXXFLAGS" + LIBS="$ac_save_LIBS" + ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + + + ac_fn_c_check_header_mongrel "$LINENO" "locale.h" "ac_cv_header_locale_h" "$ac_includes_default" if test "x$ac_cv_header_locale_h" = x""yes; then : diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 96ff16f..f59981c 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -218,6 +218,9 @@ GLIBCXX_ENABLE_LIBSTDCXX_TIME # Check for tmpnam which is obsolescent in POSIX.1-2008 GLIBCXX_CHECK_TMPNAM +# For pthread_cond_timedwaitonclock_np +GLIBCXX_CHECK_PTHREAD_COND_TIMEDWAITONCLOCK_NP + AC_LC_MESSAGES # For hardware_concurrency diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable index f7da017..883af12 100644 --- a/libstdc++-v3/include/std/condition_variable +++ b/libstdc++-v3/include/std/condition_variable @@ -35,6 +35,7 @@ # include <bits/c++0x_warning.h> #else +#include <bits/c++config.h> #include <chrono> #include <mutex> #include <ext/concurrence.h> @@ -63,7 +64,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// condition_variable class condition_variable { - typedef chrono::system_clock __clock_t; +#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP) + typedef chrono::steady_clock __steady_clock_t; + typedef chrono::steady_clock __best_clock_t; +#else + typedef chrono::system_clock __best_clock_t; +#endif + typedef chrono::system_clock __system_clock_t; typedef __gthread_cond_t __native_type; #ifdef __GTHREAD_COND_INIT @@ -98,10 +105,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait(__lock); } +#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP) template<typename _Duration> cv_status wait_until(unique_lock<mutex>& __lock, - const chrono::time_point<__clock_t, _Duration>& __atime) + const chrono::time_point<__steady_clock_t, _Duration>& __atime) + { return __wait_until_impl(__lock, __atime); } +#endif + + template<typename _Duration> + cv_status + wait_until(unique_lock<mutex>& __lock, + const chrono::time_point<__system_clock_t, _Duration>& __atime) { return __wait_until_impl(__lock, __atime); } template<typename _Clock, typename _Duration> @@ -109,9 +124,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION wait_until(unique_lock<mutex>& __lock, const chrono::time_point<_Clock, _Duration>& __atime) { - // DR 887 - Sync unknown clock to known clock. const typename _Clock::time_point __c_entry = _Clock::now(); - const __clock_t::time_point __s_entry = __clock_t::now(); + const __best_clock_t::time_point __s_entry = __best_clock_t::now(); const auto __delta = __atime - __c_entry; const auto __s_atime = __s_entry + __delta; @@ -134,24 +148,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION cv_status wait_for(unique_lock<mutex>& __lock, const chrono::duration<_Rep, _Period>& __rtime) - { return wait_until(__lock, __clock_t::now() + __rtime); } + { return wait_until(__lock, __best_clock_t::now() + __rtime); } template<typename _Rep, typename _Period, typename _Predicate> bool wait_for(unique_lock<mutex>& __lock, const chrono::duration<_Rep, _Period>& __rtime, _Predicate __p) - { return wait_until(__lock, __clock_t::now() + __rtime, std::move(__p)); } + { return wait_until(__lock, __best_clock_t::now() + __rtime, std::move(__p)); } native_handle_type native_handle() { return &_M_cond; } private: +#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP) + template<typename _Dur> + cv_status + __wait_until_impl(unique_lock<mutex>& __lock, + const chrono::time_point<__steady_clock_t, _Dur>& __atime) + { + auto __s = chrono::time_point_cast<chrono::seconds>(__atime); + auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); + + __gthread_time_t __ts = + { + static_cast<std::time_t>(__s.time_since_epoch().count()), + static_cast<long>(__ns.count()) + }; + + pthread_cond_timedwaitonclock_np(&_M_cond, __lock.mutex()->native_handle(), + CLOCK_MONOTONIC, + &__ts); + + return (__steady_clock_t::now() < __atime + ? cv_status::no_timeout : cv_status::timeout); + } +#endif template<typename _Dur> cv_status __wait_until_impl(unique_lock<mutex>& __lock, - const chrono::time_point<__clock_t, _Dur>& __atime) + const chrono::time_point<__system_clock_t, _Dur>& __atime) { auto __s = chrono::time_point_cast<chrono::seconds>(__atime); auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s); @@ -165,7 +202,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __gthread_cond_timedwait(&_M_cond, __lock.mutex()->native_handle(), &__ts); - return (__clock_t::now() < __atime + return (__system_clock_t::now() < __atime ? cv_status::no_timeout : cv_status::timeout); } }; @@ -185,7 +222,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Like above, but mutex is not required to have try_lock. class condition_variable_any { +#if defined(_GLIBCXX_USE_PTHREAD_COND_TIMEDWAITONCLOCK_NP) + typedef chrono::steady_clock __clock_t; +#else typedef chrono::system_clock __clock_t; +#endif condition_variable _M_cond; shared_ptr<mutex> _M_mutex; diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc index 04945f1..6687e9c 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc @@ -27,6 +27,7 @@ #include <system_error> #include <testsuite_hooks.h> +template <typename ClockType> void test01() { bool test __attribute__((unused)) = true; @@ -38,10 +39,10 @@ void test01() std::mutex m; std::unique_lock<std::mutex> l(m); - auto then = std::chrono::steady_clock::now(); + auto then = ClockType::now(); std::cv_status result = c1.wait_until(l, then + ms); VERIFY( result == std::cv_status::timeout ); - VERIFY( (std::chrono::steady_clock::now() - then) >= ms ); + VERIFY( (ClockType::now() - then) >= ms ); VERIFY( l.owns_lock() ); } catch (const std::system_error& e) @@ -54,8 +55,28 @@ void test01() } } +/* User defined clock that ticks in two-thousandths of a second + forty-two minutes ahead of steady_clock. */ +struct user_defined_clock +{ + typedef std::chrono::steady_clock::rep rep; + typedef std::ratio<1,2000> period; + typedef std::chrono::duration<rep, period> duration; + typedef std::chrono::time_point<user_defined_clock> time_point; + + const bool is_steady = true; + + static time_point now() noexcept + { + using namespace std::chrono; + return time_point(duration_cast<duration>(steady_clock::now().time_since_epoch()) + minutes(42)); + } +}; + int main() { - test01(); + test01<std::chrono::steady_clock>(); + test01<std::chrono::system_clock>(); + test01<user_defined_clock>(); return 0; } diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc index 75d8ac3..2b42dca 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable_any/members/2.cc @@ -52,6 +52,7 @@ struct Mutex }; +template <typename ClockType> void test01() { bool test __attribute__((unused)) = true; @@ -63,10 +64,10 @@ void test01() Mutex m; m.lock(); - auto then = std::chrono::steady_clock::now(); + auto then = ClockType::now(); std::cv_status result = c1.wait_until(m, then + ms); VERIFY( result == std::cv_status::timeout ); - VERIFY( (std::chrono::steady_clock::now() - then) >= ms ); + VERIFY( (ClockType::now() - then) >= ms ); VERIFY( m.locked ); } catch (const std::system_error& e) @@ -79,8 +80,28 @@ void test01() } } +/* User defined clock that ticks in two-thousandths of a second + forty-two minutes ahead of steady_clock. */ +struct user_defined_clock +{ + typedef std::chrono::steady_clock::rep rep; + typedef std::ratio<1,2000> period; + typedef std::chrono::duration<rep, period> duration; + typedef std::chrono::time_point<user_defined_clock> time_point; + + const bool is_steady = true; + + static time_point now() noexcept + { + using namespace std::chrono; + return time_point(duration_cast<duration>(steady_clock::now().time_since_epoch()) + minutes(42)); + } +}; + int main() { - test01(); + test01<std::chrono::steady_clock>(); + test01<std::chrono::system_clock>(); + test01<user_defined_clock>(); return 0; } -- 2.1.4