Handle wraparound of PRIntervalTime, plus crucial fixes thanks to Ere Maijala <ere@atp.fi> (138791, r=rjesup, sr=waterson).

xpcom/threads/TimerThread.cpp

@@ -192,7 +192,7 @@ NS_IMETHODIMP TimerThread::Run()
     if (mTimers.Count() > 0) {
       timer = NS_STATIC_CAST(nsTimerImpl*, mTimers[0]);
 
-      if (now >= timer->mTimeout + mTimeoutAdjustment) {
+      if (!TIMER_LESS_THAN(now, timer->mTimeout + mTimeoutAdjustment)) {
   next:
         // NB: AddRef before the Release under RemoveTimerInternal to avoid
         // mRefCnt passing through zero, in case all other refs than the one
@@ -242,7 +242,7 @@ NS_IMETHODIMP TimerThread::Run()
 
       // Don't wait at all (even for PR_INTERVAL_NO_WAIT) if the next timer is
       // due now or overdue.
-      if (now >= timeout)
+      if (!TIMER_LESS_THAN(now, timeout))
         goto next;
       waitFor = timeout - now;
     }
@@ -270,11 +270,12 @@ nsresult TimerThread::AddTimer(nsTimerImpl *aTimer)
 {
   nsAutoLock lock(mLock);
 
-  /* add the timer from our list */
+  // Add the timer to our list.
   PRInt32 i = AddTimerInternal(aTimer);
   if (i < 0)
     return NS_ERROR_OUT_OF_MEMORY;
 
+  // Awaken the timer thread.
   if (mCondVar && mWaiting && i == 0)
     PR_NotifyCondVar(mCondVar);
 
@@ -287,11 +288,16 @@ nsresult TimerThread::TimerDelayChanged(nsTimerImpl *aTimer)
 
   // Our caller has a strong ref to aTimer, so it can't go away here under
   // ReleaseTimerInternal.
-
   RemoveTimerInternal(aTimer);
-  if (AddTimerInternal(aTimer) < 0)
+
+  PRInt32 i = AddTimerInternal(aTimer);
+  if (i < 0)
     return NS_ERROR_OUT_OF_MEMORY;
 
+  // Awaken the timer thread.
+  if (mCondVar && mWaiting && i == 0)
+    PR_NotifyCondVar(mCondVar);
+
   return NS_OK;
 }
 
@@ -309,6 +315,10 @@ nsresult TimerThread::RemoveTimer(nsTimerImpl *aTimer)
   if (!RemoveTimerInternal(aTimer))
     return NS_ERROR_NOT_AVAILABLE;
 
+  // Awaken the timer thread.
+  if (mCondVar && mWaiting)
+    PR_NotifyCondVar(mCondVar);
+
   return NS_OK;
 }
 
@@ -320,7 +330,7 @@ PRInt32 TimerThread::AddTimerInternal(nsTimerImpl *aTimer)
   for (; i < count; i++) {
     nsTimerImpl *timer = NS_STATIC_CAST(nsTimerImpl *, mTimers[i]);
 
-    if (aTimer->mTimeout < timer->mTimeout) {
+    if (TIMER_LESS_THAN(aTimer->mTimeout, timer->mTimeout)) {
       break;
     }
   }

xpcom/threads/TimerThread.h

@@ -75,7 +75,9 @@ public:
   nsCOMPtr<nsIEventQueueService> mEventQueueService;
 
 private:
-  // These two internal functions must be called from within a lock
+  // These two internal helper methods must be called while mLock is held.
+  // AddTimerInternal returns the position where the timer was added in the
+  // list, or -1 if it failed.
   PRInt32 AddTimerInternal(nsTimerImpl *aTimer);
   PRBool  RemoveTimerInternal(nsTimerImpl *aTimer);
 

xpcom/threads/nsTimerImpl.cpp

@@ -280,6 +280,11 @@ NS_IMETHODIMP nsTimerImpl::Cancel()
 
 NS_IMETHODIMP_(void) nsTimerImpl::SetDelay(PRUint32 aDelay)
 {
+  // If we're already repeating precisely, update mTimeout now so that the
+  // new delay takes effect in the future.
+  if (mTimeout != 0 && mType == NS_TYPE_REPEATING_PRECISE)
+    mTimeout = PR_IntervalNow();
+
   SetDelayInternal(aDelay);
 
   if (!mFiring && gThread)
@@ -328,7 +333,7 @@ void nsTimerImpl::Fire()
   if (mType == NS_TYPE_REPEATING_PRECISE) {
     // Precise repeating timers advance mTimeout by mDelay without fail before
     // calling Fire().
-    timeout -= mDelay;
+    timeout -= PR_MillisecondsToInterval(mDelay);
   }
   gThread->UpdateFilter(mDelay, timeout, now);
 
@@ -454,17 +459,19 @@ void nsTimerImpl::PostTimerEvent()
 
 void nsTimerImpl::SetDelayInternal(PRUint32 aDelay)
 {
+  PRIntervalTime delayInterval = PR_MillisecondsToInterval(aDelay);
+  if (delayInterval > DELAY_INTERVAL_MAX) {
+    delayInterval = DELAY_INTERVAL_MAX;
+    aDelay = PR_IntervalToMilliseconds(delayInterval);
+  }
+
   mDelay = aDelay;
 
   PRIntervalTime now = PR_IntervalNow();
-  if (mTimeout == 0 || mType == NS_TYPE_REPEATING_SLACK)
+  if (mTimeout == 0 || mType != NS_TYPE_REPEATING_PRECISE)
     mTimeout = now;
 
-  mTimeout += PR_MillisecondsToInterval(aDelay);
-
-  if (mTimeout < now) { // we overflowed
-    mTimeout = PRIntervalTime(-1);
-  }
+  mTimeout += delayInterval;
 
 #ifdef DEBUG_TIMERS
   if (PR_LOG_TEST(gTimerLog, PR_LOG_DEBUG)) {

xpcom/threads/nsTimerImpl.h

@@ -73,6 +73,21 @@ enum {
   CALLBACK_TYPE_OBSERVER  = 3
 };
 
+// Two timer deadlines must differ by less than half the PRIntervalTime domain.
+#define DELAY_INTERVAL_LIMIT    PR_BIT(8 * sizeof(PRIntervalTime) - 1)
+
+// Maximum possible delay (XXX rework to use ms rather than interval ticks).
+#define DELAY_INTERVAL_MAX      (DELAY_INTERVAL_LIMIT - 1)
+
+// Is interval-time t1 less than t2, even if t1 has wrapped PRIntervalTime?
+static inline PRBool
+TIMER_LESS_THAN(PRIntervalTime t, PRIntervalTime u)
+{
+    return (t < u)
+           ? u - t < DELAY_INTERVAL_LIMIT
+           : t - u > DELAY_INTERVAL_LIMIT;
+}
+
 class nsTimerImpl : public nsITimer, public nsIScriptableTimer
 {
 public: