Fix WaitableEvent and ConditionVariable::TimedWait to use monotonic time on non-Mac non-NaCl posix

Both use a relative time, and it's important that this relative time is consistent with the wall clock when the system time gets adjusted (e.g. NTP, tlsdate, etc.). The monotonic clock, when available, has that property. Unfortunately, by default, pthread_cond_timedwait takes the system time which doesn't have that property.

On Linux/Chrome OS, we use pthread_condattr_setclock which lets us use the monotonic clock. On Android, we use the non-portable pthread_cond_timedwait_monotonic_np which has the same effect.
Unfortunately, neither is supported by NaCl.
Mac can use the non-portable pthread_cond_timedwait_relative_np which uses a relative time.

BUG=293736
R=thakis@chromium.org

Review URL: https://codereview.chromium.org/24158005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226378 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 107 insertions, 14 deletions

diff --git a/base/synchronization/condition_variable_posix.cc b/base/synchronization/condition_variable_posix.cc
index 92b479e..e70a301 100644
--- a/base/synchronization/condition_variable_posix.cc
+++ b/base/synchronization/condition_variable_posix.cc
@@ -20,7 +20,22 @@ ConditionVariable::ConditionVariable(Lock* user_lock)
     , user_lock_(user_lock)
 #endif
 {
-  int rv = pthread_cond_init(&condition_, NULL);
+  int rv = 0;
+  // http://crbug.com/293736
+  // NaCl doesn't support monotonic clock based absolute deadlines.
+  // Android supports it through the non-standard
+  // pthread_cond_timedwait_monotonic_np.
+  // Mac can use relative time deadlines.
+#if !defined(OS_MACOSX) && !defined(OS_NACL) && !defined(OS_ANDROID)
+  pthread_condattr_t attrs;
+  rv = pthread_condattr_init(&attrs);
+  DCHECK_EQ(0, rv);
+  pthread_condattr_setclock(&attrs, CLOCK_MONOTONIC);
+  rv = pthread_cond_init(&condition_, &attrs);
+  pthread_condattr_destroy(&attrs);
+#else
+  rv = pthread_cond_init(&condition_, NULL);
+#endif
   DCHECK_EQ(0, rv);
 }
 
@@ -44,23 +59,48 @@ void ConditionVariable::Wait() {
 void ConditionVariable::TimedWait(const TimeDelta& max_time) {
   base::ThreadRestrictions::AssertWaitAllowed();
   int64 usecs = max_time.InMicroseconds();
+  struct timespec relative_time;
+  relative_time.tv_sec = usecs / Time::kMicrosecondsPerSecond;
+  relative_time.tv_nsec =
+      (usecs % Time::kMicrosecondsPerSecond) * Time::kNanosecondsPerMicrosecond;
+
+#if !defined(NDEBUG)
+  user_lock_->CheckHeldAndUnmark();
+#endif
 
+#if defined(OS_MACOSX)
+  int rv = pthread_cond_timedwait_relative_np(
+      &condition_, user_mutex_, &relative_time);
+#else
   // The timeout argument to pthread_cond_timedwait is in absolute time.
+  struct timespec absolute_time;
+#if defined(OS_NACL)
+  // See comment in constructor for why this is different in NaCl.
   struct timeval now;
   gettimeofday(&now, NULL);
+  absolute_time.tv_sec = now.tv_sec;
+  absolute_time.tv_nsec = now.tv_usec * Time::kNanosecondsPerMicrosecond;
+#else
+  struct timespec now;
+  clock_gettime(CLOCK_MONOTONIC, &now);
+  absolute_time.tv_sec = now.tv_sec;
+  absolute_time.tv_nsec = now.tv_nsec;
+#endif
 
-  struct timespec abstime;
-  abstime.tv_sec = now.tv_sec + (usecs / Time::kMicrosecondsPerSecond);
-  abstime.tv_nsec = (now.tv_usec + (usecs % Time::kMicrosecondsPerSecond)) *
-                    Time::kNanosecondsPerMicrosecond;
-  abstime.tv_sec += abstime.tv_nsec / Time::kNanosecondsPerSecond;
-  abstime.tv_nsec %= Time::kNanosecondsPerSecond;
-  DCHECK_GE(abstime.tv_sec, now.tv_sec);  // Overflow paranoia
+  absolute_time.tv_sec += relative_time.tv_sec;
+  absolute_time.tv_nsec += relative_time.tv_nsec;
+  absolute_time.tv_sec += absolute_time.tv_nsec / Time::kNanosecondsPerSecond;
+  absolute_time.tv_nsec %= Time::kNanosecondsPerSecond;
+  DCHECK_GE(absolute_time.tv_sec, now.tv_sec);  // Overflow paranoia
+
+#if defined(OS_ANDROID)
+  int rv = pthread_cond_timedwait_monotonic_np(
+      &condition_, user_mutex_, &absolute_time);
+#else
+  int rv = pthread_cond_timedwait(&condition_, user_mutex_, &absolute_time);
+#endif  // OS_ANDROID
+#endif  // OS_MACOSX
 
-#if !defined(NDEBUG)
-  user_lock_->CheckHeldAndUnmark();
-#endif
-  int rv = pthread_cond_timedwait(&condition_, user_mutex_, &abstime);
   DCHECK(rv == 0 || rv == ETIMEDOUT);
 #if !defined(NDEBUG)
   user_lock_->CheckUnheldAndMark();
diff --git a/base/synchronization/condition_variable_unittest.cc b/base/synchronization/condition_variable_unittest.cc
index 4230e63..65e153c 100644
--- a/base/synchronization/condition_variable_unittest.cc
+++ b/base/synchronization/condition_variable_unittest.cc
@@ -8,12 +8,14 @@
 #include <algorithm>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/lock.h"
 #include "base/synchronization/spin_wait.h"
 #include "base/threading/platform_thread.h"
+#include "base/threading/thread.h"
 #include "base/threading/thread_collision_warner.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -188,6 +190,57 @@ TEST_F(ConditionVariableTest, TimeoutTest) {
   lock.Release();
 }
 
+#if defined(OS_POSIX)
+const int kDiscontinuitySeconds = 2;
+
+void BackInTime(Lock* lock) {
+  AutoLock auto_lock(*lock);
+
+  timeval tv;
+  gettimeofday(&tv, NULL);
+  tv.tv_sec -= kDiscontinuitySeconds;
+  settimeofday(&tv, NULL);
+}
+
+// Tests that TimedWait ignores changes to the system clock.
+// Test is disabled by default, because it needs to run as root to muck with the
+// system clock.
+// http://crbug.com/293736
+TEST_F(ConditionVariableTest, DISABLED_TimeoutAcrossSetTimeOfDay) {
+  timeval tv;
+  gettimeofday(&tv, NULL);
+  tv.tv_sec += kDiscontinuitySeconds;
+  if (settimeofday(&tv, NULL) < 0) {
+    PLOG(ERROR) << "Could not set time of day. Run as root?";
+    return;
+  }
+
+  Lock lock;
+  ConditionVariable cv(&lock);
+  lock.Acquire();
+
+  Thread thread("Helper");
+  thread.Start();
+  thread.message_loop()->PostTask(FROM_HERE, base::Bind(&BackInTime, &lock));
+
+  TimeTicks start = TimeTicks::Now();
+  const TimeDelta kWaitTime = TimeDelta::FromMilliseconds(300);
+  // Allow for clocking rate granularity.
+  const TimeDelta kFudgeTime = TimeDelta::FromMilliseconds(50);
+
+  cv.TimedWait(kWaitTime + kFudgeTime);
+  TimeDelta duration = TimeTicks::Now() - start;
+
+  thread.Stop();
+  // We can't use EXPECT_GE here as the TimeDelta class does not support the
+  // required stream conversion.
+  EXPECT_TRUE(duration >= kWaitTime);
+  EXPECT_TRUE(duration <= TimeDelta::FromSeconds(kDiscontinuitySeconds));
+
+  lock.Release();
+}
+#endif
+
 
 // Suddenly got flaky on Win, see http://crbug.com/10607 (starting at
 // comment #15)
diff --git a/base/synchronization/waitable_event_posix.cc b/base/synchronization/waitable_event_posix.cc
index 714c111..fccba9d 100644
--- a/base/synchronization/waitable_event_posix.cc
+++ b/base/synchronization/waitable_event_posix.cc
@@ -159,7 +159,7 @@ void WaitableEvent::Wait() {
 
 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   base::ThreadRestrictions::AssertWaitAllowed();
-  const Time end_time(Time::Now() + max_time);
+  const TimeTicks end_time(TimeTicks::Now() + max_time);
   const bool finite_time = max_time.ToInternalValue() >= 0;
 
   kernel_->lock_.Acquire();
@@ -184,7 +184,7 @@ bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   // again before unlocking it.
 
   for (;;) {
-    const Time current_time(Time::Now());
+    const TimeTicks current_time(TimeTicks::Now());
 
     if (sw.fired() || (finite_time && current_time >= end_time)) {
       const bool return_value = sw.fired();