Bug 1157323 - Part 3: Do not allow mTimeout to change while a timer is in the queue. r=froydnj
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 05 Aug 2016 10:07:38 -0500
changeset 315361 85739737d40d01de29d2f83d9715249900ca58d9
parent 315360 f814d7c1885428f36fe418d90a70e610a224848b
child 315362 344fdfed174a97aef02c99b9738a0b38c5c6fe43
push id30748
push usercbook@mozilla.com
push dateWed, 28 Sep 2016 13:53:19 +0000
treeherdermozilla-central@8c84b7618840 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1157323
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1157323 - Part 3: Do not allow mTimeout to change while a timer is in the queue. r=froydnj 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)
 {