From 014578b875a8bb89c31ce40fbd6b37e54ec9a353 Mon Sep 17 00:00:00 2001
From: crs <cschoeneman@gmail.com>
Date: Thu, 11 Nov 2004 19:23:14 +0000
Subject: [PATCH] Fixed a serious flaw in wrapper for posix condition variable
 wait function.  Because synergy doesn't use posix cancellation, it cannot
 wake up a thread waiting on a condition variable.  So the wrapper would wake
 up periodically to test if the thread was cancelled (according to synergy's
 cancellation state) then go back to waiting.  Well the condition could be
 signalled while we're testing and be lost and we'd never return from the
 wait. So now we wake up after a maximum timeout and return to the caller. 
 The caller must check for this spurious wakeup but all callers should do this
 anyway so we're okay there.

---
 lib/arch/CArchMultithreadPosix.cpp | 90 ++++++++++--------------------
 1 file changed, 30 insertions(+), 60 deletions(-)

diff --git a/lib/arch/CArchMultithreadPosix.cpp b/lib/arch/CArchMultithreadPosix.cpp
index 64baf6af..5750b51d 100644
--- a/lib/arch/CArchMultithreadPosix.cpp
+++ b/lib/arch/CArchMultithreadPosix.cpp
@@ -213,73 +213,43 @@ bool
 CArchMultithreadPosix::waitCondVar(CArchCond cond,
 							CArchMutex mutex, double timeout)
 {
+	// we can't wait on a condition variable and also wake it up for
+	// cancellation since we don't use posix cancellation.  so we
+	// must wake up periodically to check for cancellation.  we
+	// can't simply go back to waiting after the check since the
+	// condition may have changed and we'll have lost the signal.
+	// so we have to return to the caller.  since the caller will
+	// always check for spurious wakeups the only drawback here is
+	// performance:  we're waking up a lot more than desired.
+	static const double maxCancellationLatency = 0.1;
+	if (timeout < 0.0 || timeout > maxCancellationLatency) {
+		timeout = maxCancellationLatency;
+	}
+
+	// see if we should cancel this thread
+	testCancelThread();
+
 	// get final time
 	struct timeval now;
 	gettimeofday(&now, NULL);
 	struct timespec finalTime;
-	finalTime.tv_sec  = now.tv_sec;
-	finalTime.tv_nsec = now.tv_usec * 1000;
-	if (timeout >= 0.0) {
-		const long timeout_sec  = (long)timeout;
-		const long timeout_nsec = (long)(1.0e+9 * (timeout - timeout_sec));
-		finalTime.tv_sec  += timeout_sec;
-		finalTime.tv_nsec += timeout_nsec;
-		if (finalTime.tv_nsec >= 1000000000) {
-			finalTime.tv_nsec -= 1000000000;
-			finalTime.tv_sec  += 1;
-		}
+	finalTime.tv_sec   = now.tv_sec;
+	finalTime.tv_nsec  = now.tv_usec * 1000;
+	long timeout_sec   = (long)timeout;
+	long timeout_nsec  = (long)(1.0e+9 * (timeout - timeout_sec));
+	finalTime.tv_sec  += timeout_sec;
+	finalTime.tv_nsec += timeout_nsec;
+	if (finalTime.tv_nsec >= 1000000000) {
+		finalTime.tv_nsec -= 1000000000;
+		finalTime.tv_sec  += 1;
 	}
 
-	// repeat until we reach the final time
-	int status;
-	for (;;) {
-		// get current time
-		gettimeofday(&now, NULL);
-		struct timespec endTime;
-		endTime.tv_sec  = now.tv_sec;
-		endTime.tv_nsec = now.tv_usec * 1000;
+	// wait
+	int status = pthread_cond_timedwait(&cond->m_cond,
+							&mutex->m_mutex, &finalTime);
 
-		// done if past final timeout
-		if (timeout >= 0.0) {
-			if (endTime.tv_sec > finalTime.tv_sec ||
-				(endTime.tv_sec  == finalTime.tv_sec &&
-				 endTime.tv_nsec >= finalTime.tv_nsec)) {
-				status = ETIMEDOUT;
-				break;
-			}
-		}
-
-		// compute the next timeout
-		endTime.tv_nsec += 50000000;
-		if (endTime.tv_nsec >= 1000000000) {
-			endTime.tv_nsec -= 1000000000;
-			endTime.tv_sec  += 1;
-		}
-
-		// don't wait past final timeout
-		if (timeout >= 0.0) {
-			if (endTime.tv_sec > finalTime.tv_sec ||
-				(endTime.tv_sec  == finalTime.tv_sec &&
-				 endTime.tv_nsec >= finalTime.tv_nsec)) {
-				endTime = finalTime;
-			}
-		}
-
-		// see if we should cancel this thread
-		testCancelThread();
-
-		// wait
-		status = pthread_cond_timedwait(&cond->m_cond,
-							&mutex->m_mutex, &endTime);
-
-		// check for cancel again
-		testCancelThread();
-
-		// check wait status
-		if (status != ETIMEDOUT && status != EINTR) {
-			break;
-		}
-	}
+	// check for cancel again
+	testCancelThread();
 
 	switch (status) {
 	case 0: