[v2,07/11] PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock

Message ID 71e802f5181fae2989a8bf0972e52bee361c8c1d.1571162241.git-series.mac@mcrowe.com
State New
Headers show
Series
  • timed_mutex, shared_timed_mutex: Add full steady clock support
Related show

Commit Message

Mike Crowe Oct. 15, 2019, 5:57 p.m.
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.

Fran├žois Dumont pointed out[1] a flaw in an earlier version of this patch
that revealed a hole in the test coverage, so I've added a new test that
try_lock_until acts as try_lock if the timeout has already expired.

[1] https://gcc.gnu.org/ml/libstdc++/2019-10/msg00021.html

	* 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.
	* testsuite/30_threads/recursive_timed_mutex/try_lock_until/4.cc:
	Add new test that try_lock_until behaves as try_lock if the timeout
	has already expired or exactly matches the current time.
---
 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 ++
 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc           | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 2 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc

-- 
git-series 0.9.1

Patch

diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index e06d286..a81825d 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();
+          do {
+            auto __rtime = __atime - __now;
+            if (_M_try_lock_for(__rtime))
+              return true;
+            __now = _Clock::now();
+          } while (__atime > __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 9ca656d..6ababbd 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 ef417ea..ed218d8 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>();
 }
diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc
new file mode 100644
index 0000000..cd94ede
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc
@@ -0,0 +1,68 @@ 
+// { dg-do run }
+// { dg-options "-pthread"  }
+// { dg-require-effective-target c++14 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+// Copyright (C) 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>
+#include <slow_clock.h>
+
+template <typename clock_type>
+void test()
+{
+  typedef std::timed_mutex mutex_type;
+
+  try
+    {
+      using namespace std::chrono;
+      mutex_type m;
+
+      // Confirm that try_lock_until acts like try_lock if the timeout has
+      // already passed.
+
+      // First test with a timeout that is definitely in the past.
+      VERIFY( m.try_lock_until( clock_type::now() - 1s ) );
+      m.unlock();
+
+      // Then attempt to test with a timeout that might exactly match the
+      // current time.
+      VERIFY( m.try_lock_until( clock_type::now() ) );
+      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>();
+}