Bug 1157323 - Part 3: Do not allow mTimeout to change while a timer is in the queue. draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 05 Aug 2016 10:07:38 -0500
changeset 417174 86690a6c6411d2eb6ad12017ee52b74b7db3d2fa
parent 417173 16bfb247826fffdbc508e559fade3fdbac29976e
child 417175 445af08464069f4ce400adeffedb25ada4d71f85
push id30354
push userbcampen@mozilla.com
push dateFri, 23 Sep 2016 19:02:51 +0000
bugs1157323
milestone52.0a1
Bug 1157323 - Part 3: Do not allow mTimeout to change while a timer is in the queue. MozReview-Commit-ID: 3ZyikUsix8D
xpcom/threads/TimerThread.cpp
xpcom/threads/TimerThread.h
xpcom/threads/nsTimerImpl.cpp
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -578,49 +578,22 @@ TimerThread::AddTimer(nsTimerImpl* aTime
     mNotified = true;
     mMonitor.Notify();
   }
 
   return NS_OK;
 }
 
 nsresult
-TimerThread::TimerDelayChanged(nsTimerImpl* aTimer)
-{
-  MonitorAutoLock lock(mMonitor);
-
-  // Our caller has a strong ref to aTimer, so it can't go away here under
-  // ReleaseTimerInternal.
-  RemoveTimerInternal(aTimer);
-
-  int32_t i = AddTimerInternal(aTimer);
-  if (i < 0) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  // Awaken the timer thread.
-  if (mWaiting && i == 0) {
-    mNotified = true;
-    mMonitor.Notify();
-  }
-
-  return NS_OK;
-}
-
-nsresult
 TimerThread::RemoveTimer(nsTimerImpl* aTimer)
 {
   MonitorAutoLock lock(mMonitor);
 
   // Remove the timer from our array.  Tell callers that aTimer was not found
-  // by returning NS_ERROR_NOT_AVAILABLE.  Unlike the TimerDelayChanged case
-  // immediately above, our caller may be passing a (now-)weak ref in via the
-  // aTimer param, specifically when nsTimerImpl::Release loses a race with
-  // TimerThread::Run, must wait for the mMonitor auto-lock here, and during the
-  // wait Run drops the only remaining ref to aTimer via RemoveTimerInternal.
+  // by returning NS_ERROR_NOT_AVAILABLE.
 
   if (!RemoveTimerInternal(aTimer)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // Awaken the timer thread.
   if (mWaiting) {
     mNotified = true;
--- a/xpcom/threads/TimerThread.h
+++ b/xpcom/threads/TimerThread.h
@@ -39,17 +39,16 @@ public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIRUNNABLE
   NS_DECL_NSIOBSERVER
 
   nsresult Init();
   nsresult Shutdown();
 
   nsresult AddTimer(nsTimerImpl* aTimer);
-  nsresult TimerDelayChanged(nsTimerImpl* aTimer);
   nsresult RemoveTimer(nsTimerImpl* aTimer);
 
   void DoBeforeSleep();
   void DoAfterSleep();
 
   bool IsOnTimerThread() const
   {
     return mThread == NS_GetCurrentThread();
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -350,20 +350,25 @@ nsTimerImpl::SetDelay(uint32_t aDelay)
   if (mCallbackType == CallbackType::Unknown && mType == nsITimer::TYPE_ONE_SHOT) {
     // This may happen if someone tries to re-use a one-shot timer
     // by re-setting delay instead of reinitializing the timer.
     NS_ERROR("nsITimer->SetDelay() called when the "
              "one-shot timer is not set up.");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
+  bool reAdd = false;
+  if (gThread) {
+    reAdd = NS_SUCCEEDED(gThread->RemoveTimer(this));
+  }
+
   SetDelayInternal(aDelay);
 
-  if (!mFiring && gThread) {
-    gThread->TimerDelayChanged(this);
+  if (reAdd) {
+    gThread->AddTimer(this);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsTimerImpl::GetDelay(uint32_t* aDelay)
 {