[PATCH 1/2] condition_variable: Report early wakeup of wait_until as no_timeout

[Mike Crowe]

As currently implemented, condition_variable always ultimately waits
against std::chrono::system_clock. This clock can be changed in arbitrary
ways by the user which may result in us waking up too early or too late
when measured against the caller-supplied clock.

We can't (yet) do much about waking up too late[1], but
if we wake up too early we must return cv_status::no_timeout to indicate a
spurious wakeup rather than incorrectly returning cv_status::timeout.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861
---
 libstdc++-v3/ChangeLog                                            |  7 +++++++
 libstdc++-v3/include/std/condition_variable                       |  8 +++++++-
 libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index c9cd62a..4657af7 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,10 @@
+2018-07-20  Mike Crowe <mac@mcrowe.com>
+       * include/std/condition_variable (wait_until): Only report timeout
+       if we really have timed out when measured against the
+       caller-supplied clock.
+       * testsuite/30_threads/condition_variable/members/2.cc: Add test
+       case to confirm above behaviour.
+
 2018-07-20  Jonathan Wakely  <jwakely@redhat.com>

        PR libstdc++/86595
diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index 84863a1..a2d146a 100644
--- a/libstdc++-v3/include/std/condition_variable
+++ b/libstdc++-v3/include/std/condition_variable
@@ -116,7 +116,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        const auto __delta = __atime - __c_entry;
        const auto __s_atime = __s_entry + __delta;

-       return __wait_until_impl(__lock, __s_atime);
+       // We might get a timeout when measured against __clock_t but
+       // we need to check against the caller-supplied clock to tell
+       // whether we should return a timeout.
+       if (__wait_until_impl(__lock, __s_atime) == cv_status::timeout)
+         return _Clock::now() < __atime ? cv_status::no_timeout : cv_status::timeout;
+       else
+         return cv_status::no_timeout;
       }

     template<typename _Clock, typename _Duration, typename _Predicate>
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 6f9724b..16850a4 100644
--- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
+++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
@@ -52,8 +52,60 @@ 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
+    {
+      std::condition_variable c1;
+      std::mutex m;
+      std::unique_lock<std::mutex> l(m);
+      auto const expire = slow_clock::now() + std::chrono::seconds(1);
+
+      while (slow_clock::now() < expire)
+       {
+         auto const result = c1.wait_until(l, expire);
+
+         // If wait_until returns before the timeout has expired when
+         // measured against the supplied clock, then wait_until must
+         // return no_timeout.
+         if (slow_clock::now() < expire)
+           VERIFY(result == std::cv_status::no_timeout);
+
+         // If wait_until returns timeout then the timeout must have
+         // expired.
+         if (result == std::cv_status::timeout)
+           VERIFY(slow_clock::now() >= expire);
+       }
+    }
+  catch (const std::system_error& e)
+    {
+      VERIFY( false );
+    }
+  catch (...)
+    {
+      VERIFY( false );
+    }
+}
+
 int main()
 {
   test01();
+  test01_alternate_clock();
   return 0;
 }

base-commit: 3052e4ec519e4f5456ab63f4954ae098524316ce
--
git-series 0.9.1
BrightSign considers your privacy to be very important. The emails you send to us will be protected and secured. Furthermore, we will only use your email and contact information for the reasons you sent them to us and for tracking how effectively we respond to your requests.