[PATCH 2/6] libstdc++: Fix shared_timed_mutex for negative timeouts [PR116586]

[Jonathan Wakely]

On Sun, 14 Sep 2025 at 21:21 +0100, Mike Crowe wrote:
>Passing a timeout from before the epoch to
>std::shared_timed_mutex::try_lock_until or
>std::shared_timed_mutex::try_lock_shared_until causes the POSIX rwlock
>functions to be passed an invalid negative timeout which results in them
>returning EINVAL.
>
>thread.timedmutex.requirements.general in the C++ standard says:
>> If abs_time has already passed, the function attempts to obtain
>> ownership without blocking (as if by calling try_lock()).
>
>and a time before the epoch has clearly already passed (see description
>of libstdc++/PR116586). Let's treat such negative times as being at the
>epoch so that these methods work correctly. Add test cases to prove that
>this, and potential similar problems are no longer present.
>
>    PR libstdc++-v3/116586
>
>libstdc++-v3/ChangeLog:
>
>        * include/std/shared_mutex: (shared_timed_mutex::try_lock_until,
>          shared_timed_mutex::try_lock_shared_until): Ensure that
>          negative timeout is not passed to POSIX rwlock functions.
>        * testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc:
>          New test.

In r16-4427-gfb558b78108ff2 I pushed just the new test, without the
code changes. When fixing https://gcc.gnu.org/PR122401 I found that
the new test hangs on older Glibc. I observed it on 2.28 but
presumably other versions without your clocklock extensions are
affected.

Suggested fix below ...

>
>Signed-off-by: Mike Crowe <mac@mcrowe.com>
>---
> libstdc++-v3/include/std/shared_mutex         | 20 ++++
> .../try_lock_until/116586.cc                  | 94 +++++++++++++++++++
> 2 files changed, 114 insertions(+)
> create mode 100644 libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>
>diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
>index 94c8532399d..ef0f1df5a2b 100644
>--- a/libstdc++-v3/include/std/shared_mutex
>+++ b/libstdc++-v3/include/std/shared_mutex
>@@ -529,6 +529,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = __glibcxx_rwlock_timedwrlock(&_M_rwlock, &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
> 	// the program violated the precondition.
>@@ -555,6 +560,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = pthread_rwlock_clockwrlock(&_M_rwlock, CLOCK_MONOTONIC,
> 					       &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
>@@ -605,6 +615,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret;
> 	// Unlike for lock(), we are not allowed to throw an exception so if
> 	// the maximum number of read locks has been exceeded, or we would
>@@ -645,6 +660,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	    static_cast<long>(__ns.count())
> 	  };
>
>+	if (__ts.tv_sec < 0 || __ts.tv_nsec < 0) {
>+	  __ts.tv_sec = 0;
>+	  __ts.tv_nsec = 0;
>+	}
>+
> 	int __ret = pthread_rwlock_clockrdlock(&_M_rwlock, CLOCK_MONOTONIC,
> 					       &__ts);
> 	// On self-deadlock, we just fail to acquire the lock.  Technically,
>diff --git a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>new file mode 100644
>index 00000000000..4ed650a78cf
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock_until/116586.cc
>@@ -0,0 +1,94 @@
>+// { dg-do run { target c++14 } }
>+
>+#include <chrono>
>+#include <shared_mutex>
>+#include <testsuite_hooks.h>
>+
>+namespace chrono = std::chrono;
>+
>+// thread.timedmutex.requirements.general:
>+//   If abs_time has already passed, the function attempts to obtain
>+//   ownership without blocking (as if by calling try_lock()).
>+
>+template <typename Clock>
>+void
>+test_exclusive_absolute(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  chrono::time_point<Clock> tp(offset);
>+  VERIFY(stm.try_lock_until(tp));
>+  VERIFY(!stm.try_lock_until(tp));
>+}
>+
>+template <typename Clock>
>+void
>+test_shared_absolute(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  chrono::time_point<Clock> tp(offset);
>+  VERIFY(stm.try_lock_shared_until(tp));
>+  stm.unlock_shared();
>+
>+  VERIFY(stm.try_lock_for(chrono::seconds{10}));
>+
>+  {
>+    // NPTL will give us EDEADLK if pthread_rwlock_timedrdlock() is called on
>+    // the same thread that already holds the exclusive (write) lock, so let's
>+    // arrange for a different thread to try to acquire the shared lock.
>+    auto t = std::async(std::launch::async, [&stm, tp]() {
>+	VERIFY(!stm.try_lock_shared_until(tp));
>+      });
>+  }
>+}
>+
>+// The type of clock used for the actual wait depends on whether
>+// _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK is defined. We might as well just test
>+// both steady_clock and system_clock.
>+template <typename Clock>
>+void
>+test_exclusive_relative(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  const auto d = -Clock::now().time_since_epoch() + offset;
>+  VERIFY(stm.try_lock_for(d));
>+  VERIFY(!stm.try_lock_for(d));
>+}
>+
>+template <typename Clock>
>+void
>+test_shared_relative(chrono::nanoseconds offset)
>+{
>+  std::shared_timed_mutex stm;
>+  const auto d = -Clock::now().time_since_epoch() + offset;
>+  VERIFY(stm.try_lock_shared_for(d));
>+  stm.unlock_shared();
>+  // Should complete immediately
>+  VERIFY(stm.try_lock_for(chrono::seconds{10}));
>+  VERIFY(!stm.try_lock_shared_for(d));

I think this function needs the same use of std::async as above,
because when _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK is not defined we
still use pthread_rwlock_timedrdlock and so we still get EDEADLK (and
then loop forever) here. You won't see a problem with modern Glibc
versions because I think pthread_rwlock_clockrdlock doesn't return
EDEADLK here.


>+}
>+
>+int main()
>+{
>+  // Try once with an offset that ought to result in tv_sec == 0, tv_nsec < 0
>+  // and one with an offset that ought to result in tv_sec < 0, tv_nsec == 0
>+  // for the absolute calls at least.  It's not really possible to arrange for
>+  // the relative calls to have tv_nsec == 0 due to time advancing.
>+  for (const chrono::nanoseconds offset : {
>+      // tv_sec == 0, tv_nsec == 0
>+      chrono::nanoseconds{0},
>+      // tv_sec == 0, tv_nsec < 0
>+      chrono::duration_cast<chrono::nanoseconds>(chrono::milliseconds{-10}),
>+      // tv_sec < 0
>+      chrono::duration_cast<chrono::nanoseconds>(chrono::seconds{-10})
>+    }) {
>+    test_exclusive_absolute<chrono::system_clock>(offset);
>+    test_shared_absolute<chrono::system_clock>(offset);
>+    test_exclusive_relative<chrono::system_clock>(offset);
>+    test_shared_relative<chrono::system_clock>(offset);
>+
>+    test_exclusive_absolute<chrono::steady_clock>(offset);
>+    test_shared_absolute<chrono::steady_clock>(offset);
>+    test_exclusive_relative<chrono::steady_clock>(offset);
>+    test_shared_relative<chrono::steady_clock>(offset);
>+  }
>+}
>-- 
>2.39.5
>
>