From 3746b29b8a4358d28944b0f23642ce940455c6d2 Mon Sep 17 00:00:00 2001
From: Bryan Duxbury <bryanduxbury@apache.org>
Date: Wed, 24 Aug 2011 00:35:52 +0000
Subject: [PATCH] THRIFT-1280. cpp: Improve Monitor exception-free interfaces

Patch: Mark Rabkin

git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1160944 13f79535-47bb-0310-9956-ffa450edef68
---
 lib/cpp/src/concurrency/Monitor.cpp | 88 ++++++++++++++++++++++-------
 lib/cpp/src/concurrency/Monitor.h   | 34 ++++++++++-
 2 files changed, 101 insertions(+), 21 deletions(-)

diff --git a/lib/cpp/src/concurrency/Monitor.cpp b/lib/cpp/src/concurrency/Monitor.cpp
index 2943ef70..3d6440c5 100644
--- a/lib/cpp/src/concurrency/Monitor.cpp
+++ b/lib/cpp/src/concurrency/Monitor.cpp
@@ -68,33 +68,71 @@ class Monitor::Impl {
   void lock() { mutex().lock(); }
   void unlock() { mutex().unlock(); }
 
-  void wait(int64_t timeout) const {
+  /**
+   * Exception-throwing version of waitForTimeRelative(), called simply
+   * wait(int64) for historical reasons.  Timeout is in milliseconds.
+   *
+   * If the condition occurs,  this function returns cleanly; on timeout or
+   * error an exception is thrown.
+   */
+  void wait(int64_t timeout_ms) const {
+    int result = waitForTimeRelative(timeout_ms);
+    if (result == ETIMEDOUT) {
+      // pthread_cond_timedwait has been observed to return early on
+      // various platforms, so comment out this assert.
+      //assert(Util::currentTime() >= (now + timeout));
+      throw TimedOutException();
+    } else if (result != 0) {
+      throw TException(
+        "pthread_cond_wait() or pthread_cond_timedwait() failed");
+    }
+  }
+
+  /**
+   * Waits until the specified timeout in milliseconds for the condition to
+   * occur, or waits forever if timeout_ms == 0.
+   *
+   * Returns 0 if condition occurs, ETIMEDOUT on timeout, or an error code.
+   */
+  int waitForTimeRelative(int64_t timeout_ms) const {
+    if (timeout_ms == 0LL) {
+      return waitForever();
+    }
+
+    struct timespec abstime;
+    Util::toTimespec(abstime, Util::currentTime() + timeout_ms);
+    return waitForTime(&abstime);
+  }
+
+  /**
+   * Waits until the absolute time specified using struct timespec.
+   * Returns 0 if condition occurs, ETIMEDOUT on timeout, or an error code.
+   */
+  int waitForTime(const timespec* abstime) const {
     assert(mutex_);
     pthread_mutex_t* mutexImpl =
       reinterpret_cast<pthread_mutex_t*>(mutex_->getUnderlyingImpl());
     assert(mutexImpl);
 
     // XXX Need to assert that caller owns mutex
-    assert(timeout >= 0LL);
-    if (timeout == 0LL) {
-      int iret = pthread_cond_wait(&pthread_cond_, mutexImpl);
-      assert(iret == 0);
-    } else {
-      struct timespec abstime;
-      int64_t now = Util::currentTime();
-      Util::toTimespec(abstime, now + timeout);
-      int result = pthread_cond_timedwait(&pthread_cond_,
-                                          mutexImpl,
-                                          &abstime);
-      if (result == ETIMEDOUT) {
-        // pthread_cond_timedwait has been observed to return early on
-        // various platforms, so comment out this assert.
-        //assert(Util::currentTime() >= (now + timeout));
-        throw TimedOutException();
-      }
-    }
+    return pthread_cond_timedwait(&pthread_cond_,
+                                  mutexImpl,
+                                  abstime);
   }
 
+  /**
+   * Waits forever until the condition occurs.
+   * Returns 0 if condition occurs, or an error code otherwise.
+   */
+  int waitForever() const {
+    assert(mutex_);
+    pthread_mutex_t* mutexImpl =
+      reinterpret_cast<pthread_mutex_t*>(mutex_->getUnderlyingImpl());
+    assert(mutexImpl);
+    return pthread_cond_wait(&pthread_cond_, mutexImpl);
+  }
+
+
   void notify() {
     // XXX Need to assert that caller owns mutex
     int iret = pthread_cond_signal(&pthread_cond_);
@@ -151,6 +189,18 @@ void Monitor::unlock() const { impl_->unlock(); }
 
 void Monitor::wait(int64_t timeout) const { impl_->wait(timeout); }
 
+int Monitor::waitForTime(const timespec* abstime) const {
+  return impl_->waitForTime(abstime);
+}
+
+int Monitor::waitForTimeRelative(int64_t timeout_ms) const {
+  return impl_->waitForTimeRelative(timeout_ms);
+}
+
+int Monitor::waitForever() const {
+  return impl_->waitForever();
+}
+
 void Monitor::notify() const { impl_->notify(); }
 
 void Monitor::notifyAll() const { impl_->notifyAll(); }
diff --git a/lib/cpp/src/concurrency/Monitor.h b/lib/cpp/src/concurrency/Monitor.h
index f29119d3..aa6fe939 100644
--- a/lib/cpp/src/concurrency/Monitor.h
+++ b/lib/cpp/src/concurrency/Monitor.h
@@ -66,10 +66,40 @@ class Monitor : boost::noncopyable {
 
   virtual void unlock() const;
 
-  virtual void wait(int64_t timeout=0LL) const;
-
+  /**
+   * Waits a maximum of the specified timeout in milliseconds for the condition
+   * to occur, or waits forever if timeout_ms == 0.
+   *
+   * Returns 0 if condition occurs, ETIMEDOUT on timeout, or an error code.
+   */
+  int waitForTimeRelative(int64_t timeout_ms) const;
+
+  /**
+   * Waits until the absolute time specified using struct timespec.
+   * Returns 0 if condition occurs, ETIMEDOUT on timeout, or an error code.
+   */
+  int waitForTime(const timespec* abstime) const;
+
+  /**
+   * Waits forever until the condition occurs.
+   * Returns 0 if condition occurs, or an error code otherwise.
+   */
+  int waitForever() const;
+
+  /**
+   * Exception-throwing version of waitForTimeRelative(), called simply
+   * wait(int64) for historical reasons.  Timeout is in milliseconds.
+   *
+   * If the condition occurs,  this function returns cleanly; on timeout or
+   * error an exception is thrown.
+   */
+  void wait(int64_t timeout_ms = 0LL) const;
+
+
+  /** Wakes up one thread waiting on this monitor. */
   virtual void notify() const;
 
+  /** Wakes up all waiting threads on this monitor. */
   virtual void notifyAll() const;
 
  private:
-- 
2.17.1