public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
  2019-09-28  8:45 ` [PATCH 01/11] libstdc++ testsuite: Check return value from timed_mutex::try_lock_until Mike Crowe
@ 2019-09-28  8:45 ` Mike Crowe
  2019-10-07  5:25   ` François Dumont
  2019-09-28  8:45 ` [PATCH 03/11] libstdc++ testsuite: Also test timed_mutex with steady_clock Mike Crowe
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

A non-standard clock may tick more slowly than std::chrono::steady_clock.
This means that we risk returning false early when the specified timeout
may not have expired. This can be avoided by looping until the timeout time
as reported by the non-standard clock has been reached.

Unfortunately, we have no way to tell whether the non-standard clock ticks
more quickly that std::chrono::steady_clock. If it does then we risk
returning later than would be expected, but that is unavoidable and
permitted by the standard.

	* include/std/mutex (_M_try_lock_until): Loop until the absolute
	timeout time is reached as measured against the appropriate clock.
	* testsuite/30_threads/timed_mutex/try_lock_until/3.cc:
	Also run test using slow_clock to test above fix.
	* testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc:
	Likewise.
---
 libstdc++-v3/include/std/mutex                                              | 13 +++++++++++--
 libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc |  2 ++
 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc           |  2 ++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index e06d286..bb3a41b 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -189,8 +189,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	bool
 	_M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime)
 	{
-	  auto __rtime = __atime - _Clock::now();
-	  return _M_try_lock_for(__rtime);
+          // The user-supplied clock may not tick at the same rate as
+          // steady_clock, so we must loop in order to guarantee that
+          // the timeout has expired before returning false.
+          auto __now = _Clock::now();
+          while (__atime > __now) {
+            auto __rtime = __atime - __now;
+            if (_M_try_lock_for(__rtime))
+              return true;
+            __now = _Clock::now();
+          }
+          return false;
 	}
     };
 
diff --git a/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc b/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
index 4f0b0b5..a845cbf 100644
--- a/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
+++ b/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
@@ -26,6 +26,7 @@
 #include <thread>
 #include <system_error>
 #include <testsuite_hooks.h>
+#include <slow_clock.h>
 
 template <typename clock_type>
 void test()
@@ -71,4 +72,5 @@ int main()
 {
   test<std::chrono::system_clock>();
   test<std::chrono::steady_clock>();
+  test<slow_clock>();
 }
diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc
index 69d1ea5..9f34644 100644
--- a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc
+++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc
@@ -26,6 +26,7 @@
 #include <thread>
 #include <system_error>
 #include <testsuite_hooks.h>
+#include <slow_clock.h>
 
 template <typename clock_type>
 void test()
@@ -71,4 +72,5 @@ int main()
 {
   test<std::chrono::system_clock>();
   test<std::chrono::steady_clock>();
+  test<slow_clock>();
 }
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 09/11] libstdc++ shared_mutex: Add full steady_clock support to shared_timed_mutex
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (3 preceding siblings ...)
  2019-09-28  8:45 ` [PATCH 05/11] PR libstdc++/78237 Add full steady_clock support to timed_mutex Mike Crowe
@ 2019-09-28  8:45 ` Mike Crowe
  2019-09-28  8:48 ` [PATCH 11/11] shared_mutex: Fix try_lock_until and try_lock_shared_until on arbitrary clock Mike Crowe
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

The pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock functions
were added to glibc in v2.30. They have also been added to Android
Bionic. If these functions are available in the C library then they can be
used to implement shared_timed_mutex::try_lock_until,
shared_timed_mutex::try_lock_for, shared_timed_mutex::try_lock_shared_until
and shared_timed_mutex::try_lock_shared_for so that they are no longer
unaffected by the system clock being warped. (This is the shared_mutex
equivalent of PR libstdc++/78237 for mutex.)

If the new functions are available then steady_clock is deemed to be the
"best" clock available which means that it is used for the relative
try_lock_for calls and absolute try_lock_until calls using steady_clock and
user-defined clocks. It's not possible to have
_GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK defined without
_GLIBCXX_USE_PTHREAD_RWLOCK_T, so the requirement that the clock be the
same as condition_variable is maintained. Calls explicitly using
system_clock (aka high_resolution_clock) continue to use CLOCK_REALTIME via
the old pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock
functions.

If the new functions are not available then system_clock is deemed to be
the "best" clock available which means that the previous suboptimal
behaviour remains.

	* libstdc++-v3/acinclude.m4: Define
	GLIBCXX_CHECK_PTHREAD_RWLOCK_CLOCKLOCK to check for the presence of
	both pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock.

	* config.h.in: Add _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK define.

	* configure.ac: Call GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK.

	* configure: Add generated part to detect
	pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock
	functions.

	* include/std/shared_mutex (shared_timed_mutex): Define __clock_t
	as the best clock to use for relative waits. (try_lock_until): Use
	existing try_lock_until implementation for system_clock (which
	matches __clock_t when _GLIBCCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK is
	not defined). Add new overload for steady_clock that uses
	pthread_rwlock_clockwrlock if it is available. Simplify overload
	for non-standard clock to just call try_lock_for with a relative
	timeout. (try_lock_shared_until): Likewise.
---
 libstdc++-v3/acinclude.m4             | 33 +++++++++++-
 libstdc++-v3/config.h.in              |  4 +-
 libstdc++-v3/configure                | 88 ++++++++++++++++++++++++++++-
 libstdc++-v3/configure.ac             |  3 +-
 libstdc++-v3/include/std/shared_mutex | 87 ++++++++++++++++++++++------
 5 files changed, 198 insertions(+), 17 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 9ecfa96..d860dbe 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4259,6 +4259,39 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK], [
 ])
 
 dnl
+dnl Check whether pthread_mutex_clocklock is available in <pthread.h> for std::timed_mutex to use,
+dnl and define _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK.
+dnl
+AC_DEFUN([GLIBCXX_CHECK_PTHREAD_RWLOCK_CLOCKLOCK], [
+
+  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_rwlock_clockrdlock, pthread_wlock_clockwrlock])
+  AC_CACHE_VAL(glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK, [
+    GCC_TRY_COMPILE_OR_LINK(
+      [#include <pthread.h>],
+      [pthread_rwlock_t rwl; struct timespec ts;]
+      [int n = pthread_rwlock_clockrdlock(&rwl, CLOCK_REALTIME, &ts);]
+      [int m = pthread_rwlock_clockwrlock(&rwl, CLOCK_REALTIME, &ts);],
+      [glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK=yes],
+      [glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK=no])
+  ])
+  if test $glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK = yes; then
+    AC_DEFINE(_GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK, 1, [Define if pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock are available in <pthread.h>.])
+  fi
+  AC_MSG_RESULT($glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK)
+
+  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 9670176..6935797 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -997,6 +997,10 @@
 /* Define if pthread_mutex_clocklock is available in <pthread.h>. */
 #undef _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK
 
+/* Define if pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock are
+   available in <pthread.h>. */
+#undef _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK
+
 /* 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 41df34c..0265c1a 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -21760,6 +21760,94 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
+# For pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock
+
+
+
+  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_rwlock_clockrdlock, pthread_wlock_clockwrlock" >&5
+$as_echo_n "checking for pthread_rwlock_clockrdlock, pthread_wlock_clockwrlock... " >&6; }
+  if ${glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK+:} false; 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_rwlock_t rwl; struct timespec ts;
+      int n = pthread_rwlock_clockrdlock(&rwl, CLOCK_REALTIME, &ts);
+      int m = pthread_rwlock_clockwrlock(&rwl, CLOCK_REALTIME, &ts);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK=yes
+else
+  glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK=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_rwlock_t rwl; struct timespec ts;
+      int n = pthread_rwlock_clockrdlock(&rwl, CLOCK_REALTIME, &ts);
+      int m = pthread_rwlock_clockwrlock(&rwl, CLOCK_REALTIME, &ts);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK=yes
+else
+  glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+
+fi
+
+  if test $glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK = yes; then
+
+$as_echo "#define _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK 1" >>confdefs.h
+
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK" >&5
+$as_echo "$glibcxx_cv_PTHREAD_RWLOCK_CLOCKLOCK" >&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" = xyes; then :
 
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 35d346f..f5d2768 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -226,6 +226,9 @@ GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT
 # For pthread_mutex_clocklock
 GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK
 
+# For pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock
+GLIBCXX_CHECK_PTHREAD_RWLOCK_CLOCKLOCK
+
 AC_LC_MESSAGES
 
 # For hardware_concurrency
diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
index ffd93fb..b637bdc 100644
--- a/libstdc++-v3/include/std/shared_mutex
+++ b/libstdc++-v3/include/std/shared_mutex
@@ -450,7 +450,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     using _Base = __shared_timed_mutex_base;
 
     // Must use the same clock as condition_variable for __shared_mutex_cv.
-    typedef chrono::system_clock	__clock_t;
+#ifdef _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK
+    using __clock_t = chrono::steady_clock;
+#else
+    using __clock_t = chrono::system_clock;
+#endif
 
   public:
     shared_timed_mutex() = default;
@@ -491,7 +495,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Duration>
       bool
-      try_lock_until(const chrono::time_point<__clock_t, _Duration>& __atime)
+      try_lock_until(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);
@@ -512,23 +517,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return true;
       }
 
+#ifdef _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK
+    template<typename _Duration>
+      bool
+      try_lock_until(const chrono::time_point<chrono::steady_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 =
+	  {
+	    static_cast<std::time_t>(__s.time_since_epoch().count()),
+	    static_cast<long>(__ns.count())
+	  };
+
+	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
+                                             &__ts);
+	// On self-deadlock, we just fail to acquire the lock.  Technically,
+	// the program violated the precondition.
+	if (__ret == ETIMEDOUT || __ret == EDEADLK)
+	  return false;
+	// Errors not handled: EINVAL
+	__glibcxx_assert(__ret == 0);
+	return true;
+      }
+#endif
+
     template<typename _Clock, typename _Duration>
       bool
-      try_lock_until(const chrono::time_point<_Clock, _Duration>& __abs_time)
+      try_lock_until(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 auto __delta = __abs_time - __c_entry;
-	const auto __s_atime = __s_entry + __delta;
-	return try_lock_until(__s_atime);
+        typename _Clock::time_point __now = _Clock::now();
+        auto __rtime = __atime - __now;
+        return try_lock_for(__rtime);
       }
 
     // Shared ownership
 
     template<typename _Duration>
       bool
-      try_lock_shared_until(const chrono::time_point<__clock_t,
+      try_lock_shared_until(const chrono::time_point<chrono::system_clock,
 			    _Duration>& __atime)
       {
 	auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
@@ -564,17 +593,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return true;
       }
 
+#ifdef _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK
+    template<typename _Duration>
+      bool
+      try_lock_shared_until(const chrono::time_point<chrono::steady_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 =
+	  {
+	    static_cast<std::time_t>(__s.time_since_epoch().count()),
+	    static_cast<long>(__ns.count())
+	  };
+
+	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
+                                             &__ts);
+	// On self-deadlock, we just fail to acquire the lock.  Technically,
+	// the program violated the precondition.
+	if (__ret == ETIMEDOUT || __ret == EDEADLK)
+	  return false;
+	// Errors not handled: EINVAL
+	__glibcxx_assert(__ret == 0);
+	return true;
+      }
+#endif
+
     template<typename _Clock, typename _Duration>
       bool
       try_lock_shared_until(const chrono::time_point<_Clock,
-						     _Duration>& __abs_time)
+						     _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 auto __delta = __abs_time - __c_entry;
-	const auto __s_atime = __s_entry + __delta;
-	return try_lock_shared_until(__s_atime);
+        typename _Clock::time_point __now = _Clock::now();
+        auto __rtime = __atime - __now;
+        return try_lock_shared_for(__rtime);
       }
 
 #else // ! (_GLIBCXX_USE_PTHREAD_RWLOCK_T && _GTHREAD_USE_MUTEX_TIMEDLOCK)
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support
@ 2019-09-28  8:45 Mike Crowe
  2019-09-28  8:45 ` [PATCH 01/11] libstdc++ testsuite: Check return value from timed_mutex::try_lock_until Mike Crowe
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

glibc v2.30 added the pthread_mutex_clocklock,
pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock
functions. These accept CLOCK_MONOTONIC, so they can be used to
implement proper steady_clock support in timed_mutex,
recursive_timed_mutex and shared_timed_mutex that is immune to the
system clock being warped.

Unfortunately we can't warp the system clock in the testsuite, so it's
not possible to automatically ensure that the system_clock test cases
result in a wait on CLOCK_REALTIME and steady_clock test cases result
in a wait on CLOCK_MONOTONIC. It's recommended to run the test under
strace(1) and check whether the expected futex calls are made by glibc
or ltrace(1) and check whether the expected pthread calls are
made. The easiest way to do this is to copy and paste the line used to
build the test from the output of running the tests (for example):

 make check-target-libstdc++-v3 RUNTESTFLAGS="conformance.exp=30_threads/shared_mutex/* -v -v"

to compile the test with only the tests for one clock enabled and then
run it as:

 strace -f ./1.exe

You should see calls to:

 futex(..., FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, ...)

with large values of tv_sec when using system_clock and calls to:

 futex(..., FUTEX_WAIT_BITSET_PRIVATE, ...)

Alternatively, you can use:

 ltrace -f ./1.exe

and look for calls to pthread_mutex_clocklock,
pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock with a
parameter of 1 for CLOCK_MONOTONIC and with values of tv_sec based on
the system uptime when using steady_clock.

This series also adds some extra tests and fixes some other minor
problems with the existing implementation and tests.

Mike Crowe (11):
  libstdc++ testsuite: Check return value from timed_mutex::try_lock_until
  libstdc++ testsuite: Add timed_mutex::try_lock_until test
  libstdc++ testsuite: Also test timed_mutex with steady_clock
  libstdc++ testsuite: Also test unique_lock::try_lock_until with steady_clock
  PR libstdc++/78237 Add full steady_clock support to timed_mutex
  libstdc++ testsuite: Move slow_clock to its own header
  PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock
  libstdc++ testsuite: Also test shared_timed_mutex with steady_clock
  libstdc++ shared_mutex: Add full steady_clock support to shared_timed_mutex
  libstdc++ timed_mutex: Ensure that try_lock_for waits for long enough
  shared_mutex: Fix try_lock_until and try_lock_shared_until on arbitrary clock

 libstdc++-v3/acinclude.m4                                                   |  64 +++++++++++++++++++++++++++-
 libstdc++-v3/config.h.in                                                    |   7 +++-
 libstdc++-v3/configure                                                      | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 libstdc++-v3/configure.ac                                                   |   6 +++-
 libstdc++-v3/include/std/mutex                                              |  60 +++++++++++++++++++++----
 libstdc++-v3/include/std/shared_mutex                                       | 117 +++++++++++++++++++++++++++++++++++++++++---------
 libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc           |  17 +-------
 libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc |  76 ++++++++++++++++++++++++++++++++-
 libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc          |  87 +++++++++++++++++++++++++++++++++++++-
 libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc          |  17 ++++---
 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc           |  76 ++++++++++++++++++++++++++++++++-
 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc       |  18 +++++---
 libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc                  |  14 ++++--
 libstdc++-v3/testsuite/util/slow_clock.h                                    |  38 ++++++++++++++++-
 14 files changed, 707 insertions(+), 60 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
 create mode 100644 libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc
 create mode 100644 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc
 create mode 100644 libstdc++-v3/testsuite/util/slow_clock.h

base-commit: dcfd68ec29a6823f75a52c934fe408ce4b4b6919
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 03/11] libstdc++ testsuite: Also test timed_mutex with steady_clock
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
  2019-09-28  8:45 ` [PATCH 01/11] libstdc++ testsuite: Check return value from timed_mutex::try_lock_until Mike Crowe
  2019-09-28  8:45 ` [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock Mike Crowe
@ 2019-09-28  8:45 ` Mike Crowe
  2019-09-28  8:45 ` [PATCH 05/11] PR libstdc++/78237 Add full steady_clock support to timed_mutex Mike Crowe
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

* testsuite/30_threads/timed_mutex/try_lock_until/57641.cc:
	  Template test functions and use them to test both steady_clock
	  and system_clock.
---
 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc
index 12ee52f..6355d8f 100644
--- a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc
+++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc
@@ -49,18 +49,26 @@ struct clock
 std::timed_mutex mx;
 bool locked = false;
 
+template <typename ClockType>
 void f()
 {
-  locked = mx.try_lock_until(clock::now() + C::milliseconds(1));
+  locked = mx.try_lock_until(ClockType::now() + C::milliseconds(1));
 }
 
-int main()
+template <typename ClockType>
+void test()
 {
   std::lock_guard<std::timed_mutex> l(mx);
-  auto start = C::system_clock::now();
-  std::thread t(f);
+  auto start = ClockType::now();
+  std::thread t(f<ClockType>);
   t.join();
-  auto stop = C::system_clock::now();
+  auto stop = ClockType::now();
   VERIFY( (stop - start) < C::seconds(9) );
   VERIFY( !locked );
 }
+
+int main()
+{
+  test<C::system_clock>();
+  test<C::steady_clock>();
+}
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/11] libstdc++ testsuite: Check return value from timed_mutex::try_lock_until
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
@ 2019-09-28  8:45 ` Mike Crowe
  2019-09-28  8:45 ` [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock Mike Crowe
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

* testsuite/30_threads/unique_lock/locking/4.cc: Wrap call to
            timed_mutex::try_lock_until in VERIFY macro to check its return
            value.
---
 libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc b/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc
index fe0935f..f10cd3f 100644
--- a/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc
+++ b/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc
@@ -40,7 +40,7 @@ int main()
 
       try
 	{
-	  l.try_lock_until(t);
+	  VERIFY( l.try_lock_until(t) );
 	}
       catch(const std::system_error&)
 	{
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 05/11] PR libstdc++/78237 Add full steady_clock support to timed_mutex
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (2 preceding siblings ...)
  2019-09-28  8:45 ` [PATCH 03/11] libstdc++ testsuite: Also test timed_mutex with steady_clock Mike Crowe
@ 2019-09-28  8:45 ` Mike Crowe
  2019-09-28  8:45 ` [PATCH 09/11] libstdc++ shared_mutex: Add full steady_clock support to shared_timed_mutex Mike Crowe
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

The pthread_mutex_clocklock function is available in glibc since the 2.30
release. If this function is available in the C library it can be used to
fix PR libstdc++/78237 by supporting steady_clock properly with
timed_mutex.

This means that code using timed_mutex::try_lock_for or
timed_mutex::wait_until with steady_clock is no longer subject to timing
out early or potentially waiting for much longer if the system clock is
warped at an inopportune moment.

If pthread_mutex_clocklock is available then steady_clock is deemed to be
the "best" clock available which means that it is used for the relative
try_lock_for calls and absolute try_lock_until calls using steady_clock and
user-defined clocks. Calls explicitly using system_clock (aka
high_resolution_clock) continue to use CLOCK_REALTIME via
__gthread_cond_timedwait.

If pthread_mutex_clocklock is not available then system_clock is deemed to
be the "best" clock available which means that the previous suboptimal
behaviour remains.

	* acinclude.m4: Define GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK to
        detect presence of pthread_mutex_clocklock function.

	* config.h.in: Add _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK define.

	* configure.ac: Call GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK.

	* configure: Add generated part to detect pthread_mutex_clocklock
        function.

	* include/std/mutex (__timed_mutex_impl): Remove unnecessary
	__clock_t. (_M_try_lock_for) Use best clock to
	turn relative timeout into absolute timeout. (_M_try_lock_until)
	Keep existing implementation for system_clock. Add new
	implementation for steady_clock that calls _M_clocklock. Modify
	overload for user-defined clock to use a relative wait so that it
	automatically uses the best clock.

	* include/std/mutex (timed_mutex): If pthread_mutex_clocklock is
	available, add _M_clocklock method that calls it.

	* include/std/mutex (recursive_timed_mutex): Likewise.
---
 libstdc++-v3/acinclude.m4      | 31 +++++++++++++-
 libstdc++-v3/config.h.in       |  3 +-
 libstdc++-v3/configure         | 82 +++++++++++++++++++++++++++++++++++-
 libstdc++-v3/configure.ac      |  3 +-
 libstdc++-v3/include/std/mutex | 49 +++++++++++++++++----
 5 files changed, 160 insertions(+), 8 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index ad2cb01..9ecfa96 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4228,6 +4228,37 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT], [
 ])
 
 dnl
+dnl Check whether pthread_mutex_clocklock is available in <pthread.h> for std::timed_mutex to use,
+dnl and define _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK.
+dnl
+AC_DEFUN([GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK], [
+
+  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_mutex_clocklock])
+  AC_CACHE_VAL(glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK, [
+    GCC_TRY_COMPILE_OR_LINK(
+      [#include <pthread.h>],
+      [pthread_mutex_t mutex; struct timespec ts; int n = pthread_mutex_clocklock(&mutex, CLOCK_REALTIME, &ts);],
+      [glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=yes],
+      [glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=no])
+  ])
+  if test $glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK = yes; then
+    AC_DEFINE(_GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK, 1, [Define if pthread_mutex_clocklock is available in <pthread.h>.])
+  fi
+  AC_MSG_RESULT($glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK)
+
+  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 3d13402..9670176 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -994,6 +994,9 @@
 /* Define if pthread_cond_clockwait is available in <pthread.h>. */
 #undef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
 
+/* Define if pthread_mutex_clocklock is available in <pthread.h>. */
+#undef _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK
+
 /* 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 e646c41..41df34c 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -21677,6 +21677,88 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
+# For pthread_mutex_clocklock
+
+
+
+  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_mutex_clocklock" >&5
+$as_echo_n "checking for pthread_mutex_clocklock... " >&6; }
+  if ${glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK+:} false; 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; struct timespec ts; int n = pthread_mutex_clocklock(&mutex, CLOCK_REALTIME, &ts);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=yes
+else
+  glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=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; struct timespec ts; int n = pthread_mutex_clocklock(&mutex, CLOCK_REALTIME, &ts);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=yes
+else
+  glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+
+fi
+
+  if test $glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK = yes; then
+
+$as_echo "#define _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK 1" >>confdefs.h
+
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK" >&5
+$as_echo "$glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK" >&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" = xyes; then :
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index ad4ae0c..35d346f 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -223,6 +223,9 @@ GLIBCXX_CHECK_TMPNAM
 # For pthread_cond_clockwait
 GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT
 
+# For pthread_mutex_clocklock
+GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK
+
 AC_LC_MESSAGES
 
 # For hardware_concurrency
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index 981b672..e06d286 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -134,22 +134,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     class __timed_mutex_impl
     {
     protected:
-      typedef chrono::high_resolution_clock 	__clock_t;
-
       template<typename _Rep, typename _Period>
 	bool
 	_M_try_lock_for(const chrono::duration<_Rep, _Period>& __rtime)
 	{
-	  using chrono::steady_clock;
-	  auto __rt = chrono::duration_cast<steady_clock::duration>(__rtime);
-	  if (ratio_greater<steady_clock::period, _Period>())
+#if _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK
+          using __clock = chrono::steady_clock;
+#else
+          using __clock = chrono::system_clock;
+#endif
+
+	  auto __rt = chrono::duration_cast<__clock::duration>(__rtime);
+	  if (ratio_greater<__clock::period, _Period>())
 	    ++__rt;
-	  return _M_try_lock_until(steady_clock::now() + __rt);
+	  return _M_try_lock_until(__clock::now() + __rt);
 	}
 
       template<typename _Duration>
 	bool
-	_M_try_lock_until(const chrono::time_point<__clock_t,
+        _M_try_lock_until(const chrono::time_point<chrono::system_clock,
 						   _Duration>& __atime)
 	{
 	  auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
@@ -163,12 +166,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return static_cast<_Derived*>(this)->_M_timedlock(__ts);
 	}
 
+#ifdef _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK
+      template<typename _Duration>
+        bool
+	_M_try_lock_until(const chrono::time_point<chrono::steady_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 = {
+	    static_cast<std::time_t>(__s.time_since_epoch().count()),
+	    static_cast<long>(__ns.count())
+	  };
+
+          return static_cast<_Derived*>(this)->_M_clocklock(CLOCK_MONOTONIC,
+                                                            __ts);
+	}
+#endif
+
       template<typename _Clock, typename _Duration>
 	bool
 	_M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime)
 	{
 	  auto __rtime = __atime - _Clock::now();
-	  return _M_try_lock_until(__clock_t::now() + __rtime);
+	  return _M_try_lock_for(__rtime);
 	}
     };
 
@@ -229,6 +251,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       bool
       _M_timedlock(const __gthread_time_t& __ts)
       { return !__gthread_mutex_timedlock(&_M_mutex, &__ts); }
+#if _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK
+      bool
+      _M_clocklock(clockid_t clockid, const __gthread_time_t& __ts)
+      { return !pthread_mutex_clocklock(&_M_mutex, clockid, &__ts); }
+#endif
   };
 
   /// recursive_timed_mutex
@@ -289,6 +316,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       bool
       _M_timedlock(const __gthread_time_t& __ts)
       { return !__gthread_recursive_mutex_timedlock(&_M_mutex, &__ts); }
+
+#ifdef _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK
+      bool
+      _M_clocklock(clockid_t clockid, const __gthread_time_t& __ts)
+      { return !pthread_mutex_clocklock(&_M_mutex, clockid, &__ts); }
+#endif
   };
 
 #else // !_GTHREAD_USE_MUTEX_TIMEDLOCK
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 04/11] libstdc++ testsuite: Also test unique_lock::try_lock_until with steady_clock
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (7 preceding siblings ...)
  2019-09-28  8:48 ` [PATCH 08/11] libstdc++ testsuite: Also test shared_timed_mutex with steady_clock Mike Crowe
@ 2019-09-28  8:48 ` Mike Crowe
  2019-09-28  8:48 ` [PATCH 06/11] libstdc++ testsuite: Move slow_clock to its own header Mike Crowe
  2019-09-28  8:48 ` [PATCH 10/11] libstdc++ timed_mutex: Ensure that try_lock_for waits for long enough Mike Crowe
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:48 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

* testsuite/30_threads/unique_lock/locking/4.cc: Template test
            functions so they can be used to test both steady_clock and
            system_clock.
---
 libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc | 12 +++++--
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc b/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc
index f10cd3f..1c544be 100644
--- a/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc
+++ b/libstdc++-v3/testsuite/30_threads/unique_lock/locking/4.cc
@@ -26,17 +26,18 @@
 #include <system_error>
 #include <testsuite_hooks.h>
 
-int main()
+template <typename clock_type>
+void test()
 {
   typedef std::timed_mutex mutex_type;
   typedef std::unique_lock<mutex_type> lock_type;
-  typedef std::chrono::system_clock clock_type;
 
   try 
     {
       mutex_type m;
       lock_type l(m, std::defer_lock);
-      clock_type::time_point t = clock_type::now() + std::chrono::seconds(1);
+      const typename clock_type::time_point t = clock_type::now()
+        + std::chrono::seconds(1);
 
       try
 	{
@@ -61,6 +62,11 @@ int main()
     {
       VERIFY( false );
     }
+}
 
+int main()
+{
+  test<std::chrono::system_clock>();
+  test<std::chrono::steady_clock>();
   return 0;
 }
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 10/11] libstdc++ timed_mutex: Ensure that try_lock_for waits for long enough
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (9 preceding siblings ...)
  2019-09-28  8:48 ` [PATCH 06/11] libstdc++ testsuite: Move slow_clock to its own header Mike Crowe
@ 2019-09-28  8:48 ` Mike Crowe
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:48 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

The user-defined clock used with shared_mutex::try_lock_for and
shared_mutex::try_lock_shared_for may have higher precision than
__clock_t. We may need to round the duration up to ensure that the timeout
is long enough. (See __timed_mutex_impl::_M_try_lock_for)

	* include/std/shared_mutex: (try_lock_for) Round up wait duration
          if necessary. (try_lock_shared_for) Likewise.
---
 libstdc++-v3/include/std/shared_mutex | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
index b637bdc..6e67324 100644
--- a/libstdc++-v3/include/std/shared_mutex
+++ b/libstdc++-v3/include/std/shared_mutex
@@ -471,9 +471,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Rep, typename _Period>
       bool
-      try_lock_for(const chrono::duration<_Rep, _Period>& __rel_time)
+      try_lock_for(const chrono::duration<_Rep, _Period>& __rtime)
       {
-	return try_lock_until(__clock_t::now() + __rel_time);
+        auto __rt = chrono::duration_cast<__clock_t::duration>(__rtime);
+        if (ratio_greater<__clock_t::period, _Period>())
+          ++__rt;
+	return try_lock_until(__clock_t::now() + __rt);
       }
 
     // Shared ownership
@@ -484,9 +487,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Rep, typename _Period>
       bool
-      try_lock_shared_for(const chrono::duration<_Rep, _Period>& __rel_time)
+      try_lock_shared_for(const chrono::duration<_Rep, _Period>& __rtime)
       {
-	return try_lock_shared_until(__clock_t::now() + __rel_time);
+        auto __rt = chrono::duration_cast<__clock_t::duration>(__rtime);
+        if (ratio_greater<__clock_t::period, _Period>())
+          ++__rt;
+	return try_lock_shared_until(__clock_t::now() + __rt);
       }
 
 #if _GLIBCXX_USE_PTHREAD_RWLOCK_T && _GTHREAD_USE_MUTEX_TIMEDLOCK
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 11/11] shared_mutex: Fix try_lock_until and try_lock_shared_until on arbitrary clock
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (4 preceding siblings ...)
  2019-09-28  8:45 ` [PATCH 09/11] libstdc++ shared_mutex: Add full steady_clock support to shared_timed_mutex Mike Crowe
@ 2019-09-28  8:48 ` Mike Crowe
  2019-09-28  8:48 ` [PATCH 02/11] libstdc++ testsuite: Add timed_mutex::try_lock_until test Mike Crowe
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:48 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

This is the equivalent to PR libstdc++/91906, but for shared_mutex.

A non-standard clock may tick more slowly than std::chrono::steady_clock.
This means that we risk returning false early when the specified timeout
may not have expired. This can be avoided by looping until the timeout time
as reported by the non-standard clock has been reached.

Unfortunately, we have no way to tell whether the non-standard clock ticks
more quickly that std::chrono::steady_clock. If it does then we risk
returning later than would be expected, but that is unavoidable without
waking up periodically to check, which would be rather too expensive.

	* include/std/shared_mutex (try_lock_until, try_lock_shared_until):
	Loop until the absolute timeout time is reached as measured against
	the appropriate clock.  *
	testsuite/30_threads/shared_mutex/try_lock_until/1.cc: New
	file. Test shared_mutex::try_lock_until and
	shared_mutex::try_lock_shared_until timeouts against various
	clocks.
---
 libstdc++-v3/include/std/shared_mutex                              | 28 ++++++++++++++++++-----
 libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 109 insertions(+), 6 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc

diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
index 6e67324..40d45a1 100644
--- a/libstdc++-v3/include/std/shared_mutex
+++ b/libstdc++-v3/include/std/shared_mutex
@@ -554,9 +554,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       bool
       try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime)
       {
-        typename _Clock::time_point __now = _Clock::now();
-        auto __rtime = __atime - __now;
-        return try_lock_for(__rtime);
+        // The user-supplied clock may not tick at the same rate as
+        // steady_clock, so we must loop in order to guarantee that
+        // the timeout has expired before returning false.
+	typename _Clock::time_point __now = _Clock::now();
+        while (__atime > __now) {
+          auto __rtime = __atime - __now;
+          if (try_lock_for(__rtime))
+            return true;
+          __now = _Clock::now();
+        }
+        return false;
       }
 
     // Shared ownership
@@ -631,9 +639,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       try_lock_shared_until(const chrono::time_point<_Clock,
 						     _Duration>& __atime)
       {
-        typename _Clock::time_point __now = _Clock::now();
-        auto __rtime = __atime - __now;
-        return try_lock_shared_for(__rtime);
+        // The user-supplied clock may not tick at the same rate as
+        // steady_clock, so we must loop in order to guarantee that
+        // the timeout has expired before returning false.
+	typename _Clock::time_point __now = _Clock::now();
+        while (__atime > __now) {
+          auto __rtime = __atime - __now;
+          if (try_lock_shared_for(__rtime))
+            return true;
+          __now = _Clock::now();
+        }
+        return false;
       }
 
 #else // ! (_GLIBCXX_USE_PTHREAD_RWLOCK_T && _GTHREAD_USE_MUTEX_TIMEDLOCK)
diff --git a/libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc b/libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc
new file mode 100644
index 0000000..77baad9
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/shared_mutex/try_lock_until/1.cc
@@ -0,0 +1,87 @@
+// { dg-do run }
+// { dg-options "-pthread"  }
+// { dg-require-effective-target c++14 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+// Copyright (C) 2013-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/>.
+
+
+#include <shared_mutex>
+#include <thread>
+#include <system_error>
+#include <testsuite_hooks.h>
+#include <slow_clock.h>
+
+template <typename clock_type>
+void test()
+{
+  typedef std::shared_timed_mutex mutex_type;
+
+  try
+    {
+      mutex_type m;
+      m.lock();
+      bool b;
+
+      std::thread t([&] {
+        try
+          {
+            using namespace std::chrono;
+            const auto timeout = 100ms;
+
+	    {
+	      const auto start = clock_type::now();
+	      const auto b = m.try_lock_until(start + timeout);
+	      const auto t = clock_type::now() - start;
+	      VERIFY( !b );
+	      VERIFY( t >= timeout );
+	    }
+
+	    {
+	      const auto start = clock_type::now();
+	      const auto b = m.try_lock_shared_until(start + timeout);
+	      const auto t = clock_type::now() - start;
+	      VERIFY( !b );
+	      VERIFY( t >= timeout );
+	    }
+          }
+        catch (const std::system_error& e)
+          {
+            VERIFY( false );
+          }
+	});
+      t.join();
+      m.unlock();
+    }
+  catch (const std::system_error& e)
+    {
+      VERIFY( false );
+    }
+  catch (...)
+    {
+      VERIFY( false );
+    }
+}
+
+int main()
+{
+  test<std::chrono::system_clock>();
+  test<std::chrono::steady_clock>();
+  test<slow_clock>();
+}
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 02/11] libstdc++ testsuite: Add timed_mutex::try_lock_until test
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (5 preceding siblings ...)
  2019-09-28  8:48 ` [PATCH 11/11] shared_mutex: Fix try_lock_until and try_lock_shared_until on arbitrary clock Mike Crowe
@ 2019-09-28  8:48 ` Mike Crowe
  2019-09-28  8:48 ` [PATCH 08/11] libstdc++ testsuite: Also test shared_timed_mutex with steady_clock Mike Crowe
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:48 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

I was unable to find an existing tests for timed_mutex::try_lock_until and
recursive_timed_mutex::try_lock_until timing out. It would have been easier
to add a single templated test, but since these classes are tested in
separate directories I've created two separate tests.

	* testsuite/30_threads/timed_mutex/try_lock_until/3.cc: New
 	file. Ensure that timed_mutex::try_lock_until actually times out
 	after the specified time when using both system_clock and
 	steady_clock.

	* testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc:
          Likewise but for recursive_timed_mutex.
---
 libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc           | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 148 insertions(+)
 create mode 100644 libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
 create mode 100644 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc

diff --git a/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc b/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
new file mode 100644
index 0000000..4f0b0b5
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc
@@ -0,0 +1,74 @@
+// { dg-do run }
+// { dg-options "-pthread"  }
+// { dg-require-effective-target c++14 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+// Copyright (C) 2013-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/>.
+
+
+#include <mutex>
+#include <thread>
+#include <system_error>
+#include <testsuite_hooks.h>
+
+template <typename clock_type>
+void test()
+{
+  typedef std::recursive_timed_mutex mutex_type;
+
+  try
+    {
+      mutex_type m;
+      m.lock();
+      bool b;
+
+      std::thread t([&] {
+        try
+          {
+            using namespace std::chrono;
+            const auto timeout = 100ms;
+            const auto start = clock_type::now();
+            const auto b = m.try_lock_until(start + timeout);
+            const auto t = clock_type::now() - start;
+            VERIFY( !b );
+            VERIFY( t >= timeout );
+          }
+        catch (const std::system_error& e)
+          {
+            VERIFY( false );
+          }
+	});
+      t.join();
+      m.unlock();
+    }
+  catch (const std::system_error& e)
+    {
+      VERIFY( false );
+    }
+  catch (...)
+    {
+      VERIFY( false );
+    }
+}
+
+int main()
+{
+  test<std::chrono::system_clock>();
+  test<std::chrono::steady_clock>();
+}
diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc
new file mode 100644
index 0000000..69d1ea5
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc
@@ -0,0 +1,74 @@
+// { dg-do run }
+// { dg-options "-pthread"  }
+// { dg-require-effective-target c++14 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+// Copyright (C) 2013-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/>.
+
+
+#include <mutex>
+#include <thread>
+#include <system_error>
+#include <testsuite_hooks.h>
+
+template <typename clock_type>
+void test()
+{
+  typedef std::timed_mutex mutex_type;
+
+  try
+    {
+      mutex_type m;
+      m.lock();
+      bool b;
+
+      std::thread t([&] {
+        try
+          {
+            using namespace std::chrono;
+            const auto timeout = 100ms;
+            const auto start = clock_type::now();
+            const auto b = m.try_lock_until(start + timeout);
+            const auto t = clock_type::now() - start;
+            VERIFY( !b );
+            VERIFY( t >= timeout );
+          }
+        catch (const std::system_error& e)
+          {
+            VERIFY( false );
+          }
+	});
+      t.join();
+      m.unlock();
+    }
+  catch (const std::system_error& e)
+    {
+      VERIFY( false );
+    }
+  catch (...)
+    {
+      VERIFY( false );
+    }
+}
+
+int main()
+{
+  test<std::chrono::system_clock>();
+  test<std::chrono::steady_clock>();
+}
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 08/11] libstdc++ testsuite: Also test shared_timed_mutex with steady_clock
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (6 preceding siblings ...)
  2019-09-28  8:48 ` [PATCH 02/11] libstdc++ testsuite: Add timed_mutex::try_lock_until test Mike Crowe
@ 2019-09-28  8:48 ` Mike Crowe
  2019-09-28  8:48 ` [PATCH 04/11] libstdc++ testsuite: Also test unique_lock::try_lock_until " Mike Crowe
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:48 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

* testsuite/30_threads/shared_timed_mutex/try_lock/3.cc: Convert
          existing test to templated function so that it can be called with
          both system_clock and steady_clock.
---
 libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc
index fd21033..1cf191c 100644
--- a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc
+++ b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc
@@ -27,7 +27,8 @@
 #include <system_error>
 #include <testsuite_hooks.h>
 
-int main()
+template <typename clock_type>
+void test()
 {
   typedef std::shared_timed_mutex mutex_type;
 
@@ -42,15 +43,15 @@ int main()
           {
             using namespace std::chrono;
             auto timeout = 100ms;
-            auto start = system_clock::now();
+            auto start = clock_type::now();
             b = m.try_lock_for(timeout);
-            auto t = system_clock::now() - start;
+            auto t = clock_type::now() - start;
             VERIFY( !b );
             VERIFY( t >= timeout );
 
-            start = system_clock::now();
+            start = clock_type::now();
             b = m.try_lock_until(start + timeout);
-            t = system_clock::now() - start;
+            t = clock_type::now() - start;
             VERIFY( !b );
             VERIFY( t >= timeout );
           }
@@ -71,3 +72,9 @@ int main()
       VERIFY( false );
     }
 }
+
+int main()
+{
+  test<std::chrono::system_clock>();
+  test<std::chrono::steady_clock>();
+}
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 06/11] libstdc++ testsuite: Move slow_clock to its own header
  2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
                   ` (8 preceding siblings ...)
  2019-09-28  8:48 ` [PATCH 04/11] libstdc++ testsuite: Also test unique_lock::try_lock_until " Mike Crowe
@ 2019-09-28  8:48 ` Mike Crowe
  2019-09-28  8:48 ` [PATCH 10/11] libstdc++ timed_mutex: Ensure that try_lock_for waits for long enough Mike Crowe
  10 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-09-28  8:48 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Mike Crowe

Move slow_clock test class into a header file so that it can be used by
other tests in the future.

	* testsuite/util/slow_clock.h: New file. Move implementation of
	slow_clock test class.

	* testsuite/30_threads/condition_variable/members/2.cc: Include
	slow_clock from header.
---
 libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc | 17 +----------------
 libstdc++-v3/testsuite/util/slow_clock.h                          | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 16 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/util/slow_clock.h

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 cbac3fa..f821d3f 100644
--- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
+++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
@@ -25,6 +25,7 @@
 #include <condition_variable>
 #include <system_error>
 #include <testsuite_hooks.h>
+#include <slow_clock.h>
 
 template <typename ClockType>
 void test01()
@@ -52,22 +53,6 @@ void test01()
     }
 }
 
-struct slow_clock
-{
-  using rep = std::chrono::system_clock::rep;
-  using period = std::chrono::system_clock::period;
-  using duration = std::chrono::system_clock::duration;
-  using time_point = std::chrono::time_point<slow_clock, duration>;
-  static constexpr bool is_steady = false;
-
-  static time_point now()
-  {
-    auto real = std::chrono::system_clock::now();
-    return time_point{real.time_since_epoch() / 3};
-  }
-};
-
-
 void test01_alternate_clock()
 {
   try
diff --git a/libstdc++-v3/testsuite/util/slow_clock.h b/libstdc++-v3/testsuite/util/slow_clock.h
new file mode 100644
index 0000000..13fc94b
--- /dev/null
+++ b/libstdc++-v3/testsuite/util/slow_clock.h
@@ -0,0 +1,38 @@
+// -*- C++ -*-
+
+// Copyright (C) 2004-2019 Free Software Foundation, Inc.
+
+// 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/>.
+
+
+// A clock that ticks at a third of the speed of system_clock that can be used
+// to ensure that functions with timeouts don't erroneously return early.
+
+#include <chrono>
+
+struct slow_clock
+{
+  using rep = std::chrono::system_clock::rep;
+  using period = std::chrono::system_clock::period;
+  using duration = std::chrono::system_clock::duration;
+  using time_point = std::chrono::time_point<slow_clock, duration>;
+  static constexpr bool is_steady = false;
+
+  static time_point now()
+  {
+    auto real = std::chrono::system_clock::now();
+    return time_point{real.time_since_epoch() / 3};
+  }
+};
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock
  2019-09-28  8:45 ` [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock Mike Crowe
@ 2019-10-07  5:25   ` François Dumont
  2019-10-13 14:51     ` Mike Crowe
  0 siblings, 1 reply; 14+ messages in thread
From: François Dumont @ 2019-10-07  5:25 UTC (permalink / raw)
  To: libstdc++

Sorry if I am totally out of subject here as I don't know this part of 
the lib outside the patches I see sometimes on this mailing list.

On 9/28/19 10:44 AM, Mike Crowe wrote:
> A non-standard clock may tick more slowly than std::chrono::steady_clock.
> This means that we risk returning false early when the specified timeout
> may not have expired. This can be avoided by looping until the timeout time
> as reported by the non-standard clock has been reached.
>
> Unfortunately, we have no way to tell whether the non-standard clock ticks
> more quickly that std::chrono::steady_clock. If it does then we risk
> returning later than would be expected, but that is unavoidable and
> permitted by the standard.
>
> 	* include/std/mutex (_M_try_lock_until): Loop until the absolute
> 	timeout time is reached as measured against the appropriate clock.
> 	* testsuite/30_threads/timed_mutex/try_lock_until/3.cc:
> 	Also run test using slow_clock to test above fix.
> 	* testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc:
> 	Likewise.
> ---
>   libstdc++-v3/include/std/mutex                                              | 13 +++++++++++--
>   libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc |  2 ++
>   libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc           |  2 ++
>   3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
> index e06d286..bb3a41b 100644
> --- a/libstdc++-v3/include/std/mutex
> +++ b/libstdc++-v3/include/std/mutex
> @@ -189,8 +189,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   	bool
>   	_M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime)
>   	{
> -	  auto __rtime = __atime - _Clock::now();
> -	  return _M_try_lock_for(__rtime);
> +          // The user-supplied clock may not tick at the same rate as
> +          // steady_clock, so we must loop in order to guarantee that
> +          // the timeout has expired before returning false.
> +          auto __now = _Clock::now();

Isn't it possible here that __now == __atime ?

In this case the loop won't even be entered giving no chance for 
_M_try_lock_for to return true as it used to be done in the previous code.

So a do/while seems better to me. Or at least a __atime >= __now if the 
Standard says that false shall be returned when __atime < __now even 
without trying to lock.

> +          while (__atime > __now) {
> +            auto __rtime = __atime - __now;
> +            if (_M_try_lock_for(__rtime))
> +              return true;
> +            __now = _Clock::now();
> +          }
> +          return false;
>   	}
>       };

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock
  2019-10-07  5:25   ` François Dumont
@ 2019-10-13 14:51     ` Mike Crowe
  0 siblings, 0 replies; 14+ messages in thread
From: Mike Crowe @ 2019-10-13 14:51 UTC (permalink / raw)
  To: libstdc++

On Monday 07 October 2019 at 07:24:58 +0200, François Dumont wrote:
> On 9/28/19 10:44 AM, Mike Crowe wrote:

[snip]

> > diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
> > index e06d286..bb3a41b 100644
> > --- a/libstdc++-v3/include/std/mutex
> > +++ b/libstdc++-v3/include/std/mutex
> > @@ -189,8 +189,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >   	bool
> >   	_M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime)
> >   	{
> > -	  auto __rtime = __atime - _Clock::now();
> > -	  return _M_try_lock_for(__rtime);
> > +          // The user-supplied clock may not tick at the same rate as
> > +          // steady_clock, so we must loop in order to guarantee that
> > +          // the timeout has expired before returning false.
> > +          auto __now = _Clock::now();
> 
> Isn't it possible here that __now == __atime ?
> 
> In this case the loop won't even be entered giving no chance for
> _M_try_lock_for to return true as it used to be done in the previous code.
> 
> So a do/while seems better to me. Or at least a __atime >= __now if the
> Standard says that false shall be returned when __atime < __now even without
> trying to lock.
>
> > +          while (__atime > __now) {
> > +            auto __rtime = __atime - __now;
> > +            if (_M_try_lock_for(__rtime))
> > +              return true;
> > +            __now = _Clock::now();
> > +          }
> > +          return false;
> >   	}
> >       };

You are correct. I'll try to come up with a new test that fails due to this
flaw and check for the same in the rest of the series.

Thanks!

Mike.




> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-10-13 14:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-28  8:45 [PATCH 00/11] timed_mutex, shared_timed_mutex: Add full steady clock support Mike Crowe
2019-09-28  8:45 ` [PATCH 01/11] libstdc++ testsuite: Check return value from timed_mutex::try_lock_until Mike Crowe
2019-09-28  8:45 ` [PATCH 07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock Mike Crowe
2019-10-07  5:25   ` François Dumont
2019-10-13 14:51     ` Mike Crowe
2019-09-28  8:45 ` [PATCH 03/11] libstdc++ testsuite: Also test timed_mutex with steady_clock Mike Crowe
2019-09-28  8:45 ` [PATCH 05/11] PR libstdc++/78237 Add full steady_clock support to timed_mutex Mike Crowe
2019-09-28  8:45 ` [PATCH 09/11] libstdc++ shared_mutex: Add full steady_clock support to shared_timed_mutex Mike Crowe
2019-09-28  8:48 ` [PATCH 11/11] shared_mutex: Fix try_lock_until and try_lock_shared_until on arbitrary clock Mike Crowe
2019-09-28  8:48 ` [PATCH 02/11] libstdc++ testsuite: Add timed_mutex::try_lock_until test Mike Crowe
2019-09-28  8:48 ` [PATCH 08/11] libstdc++ testsuite: Also test shared_timed_mutex with steady_clock Mike Crowe
2019-09-28  8:48 ` [PATCH 04/11] libstdc++ testsuite: Also test unique_lock::try_lock_until " Mike Crowe
2019-09-28  8:48 ` [PATCH 06/11] libstdc++ testsuite: Move slow_clock to its own header Mike Crowe
2019-09-28  8:48 ` [PATCH 10/11] libstdc++ timed_mutex: Ensure that try_lock_for waits for long enough Mike Crowe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).