On 20/07/18 17:49 +0100, Mike Crowe wrote:
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

Committed to trunk, thanks very much.

I reformatted it slightly, to keep the line below 80 columns. The
version I committed is attached.

commit 79a8b4c1d70287ac0e668c4f2335d70d97c3002e
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Aug 1 15:39:45 2018 +0000

    Report early wakeup of condition_variable::wait_until as no_timeout
    
    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 (PR 41861), 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.
    
    2018-08-01  Mike Crowe  <m...@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.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@263224 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index 3f690c81799..c00afa2b7ae 100644
--- a/libstdc++-v3/include/std/condition_variable
+++ b/libstdc++-v3/include/std/condition_variable
@@ -117,7 +117,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	const auto __delta = __atime - __c_entry;
 	const auto __s_atime = __s_entry + __delta;
 
-	return __wait_until_impl(__lock, __s_atime);
+	if (__wait_until_impl(__lock, __s_atime) == cv_status::no_timeout)
+	  return cv_status::no_timeout;
+	// We got 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 (_Clock::now() < __atime)
+	  return cv_status::no_timeout;
+	return cv_status::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 0a44c4fa7cf..09a717801e1 100644
--- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
+++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc
@@ -51,8 +51,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;
 }

Reply via email to