Bug 420521 - Leaking nsThread and nsTimerImpl running full set of Mochitests. This fixes the last refcounted leak on OS X Mochitests! Zounds! r=brendan, sr=dbaron, a=blocker
authorjwalden@mit.edu
Mon, 10 Mar 2008 14:05:46 -0700
changeset 12852 aeb5345ab010971b5501b182e291970f91ba646c
parent 12850 1e10d63bcf39fefc58fb9ff1b8d71b7d0594dc69
child 12853 892a8012c628d1a406c30ee754b3a40be0ef94c4
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherdermozilla-central@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbrendan, dbaron, blocker
bugs420521
milestone1.9b5pre
Bug 420521 - Leaking nsThread and nsTimerImpl running full set of Mochitests. This fixes the last refcounted leak on OS X Mochitests! Zounds! r=brendan, sr=dbaron, a=blocker
xpcom/threads/TimerThread.cpp
xpcom/threads/nsTimerImpl.cpp
xpcom/threads/nsTimerImpl.h
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -272,17 +272,33 @@ NS_IMETHODIMP TimerThread::Run()
                     : -(PRInt32)PR_IntervalToMilliseconds(timer->mTimeout-now))
                   );
           }
 #endif
 
           // We are going to let the call to PostTimerEvent here handle the
           // release of the timer so that we don't end up releasing the timer
           // on the TimerThread instead of on the thread it targets.
-          timer->PostTimerEvent();
+          if (NS_FAILED(timer->PostTimerEvent())) {
+            nsrefcnt rc;
+            NS_RELEASE2(timer, rc);
+            
+            // The nsITimer interface requires that its users keep a reference
+            // to the timers they use while those timers are initialized but
+            // have not yet fired.  If this ever happens, it is a bug in the
+            // code that created and used the timer.
+            //
+            // Further, note that this should never happen even with a
+            // misbehaving user, because nsTimerImpl::Release checks for a
+            // refcount of 1 with an armed timer (a timer whose only reference
+            // is from the timer thread) and when it hits this will remove the
+            // timer from the timer thread and thus destroy the last reference,
+            // preventing this situation from occurring.
+            NS_ASSERTION(rc != 0, "destroyed timer off its target thread!");
+          }
           timer = nsnull;
 
           lock.lock();
           if (mShutdown)
             break;
 
           // Update now, as PostTimerEvent plus the locking may have taken a
           // tick or two, and we may goto next below.
@@ -324,17 +340,17 @@ NS_IMETHODIMP TimerThread::Run()
 
 nsresult TimerThread::AddTimer(nsTimerImpl *aTimer)
 {
   nsAutoLock lock(mLock);
 
   // Add the timer to our list.
   PRInt32 i = AddTimerInternal(aTimer);
   if (i < 0)
-    return NS_ERROR_FAILURE;
+    return NS_ERROR_OUT_OF_MEMORY;
 
   // Awaken the timer thread.
   if (mCondVar && mWaiting && i == 0)
     PR_NotifyCondVar(mCondVar);
 
   return NS_OK;
 }
 
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -445,28 +445,30 @@ void nsTimerImpl::Fire()
 
 class nsTimerEvent : public nsRunnable {
 public:
   NS_IMETHOD Run();
 
   nsTimerEvent(nsTimerImpl *timer, PRInt32 generation)
     : mTimer(timer), mGeneration(generation) {
     // timer is already addref'd for us
+    MOZ_COUNT_CTOR(nsTimerEvent);
   }
 
 #ifdef DEBUG_TIMERS
   PRIntervalTime mInitTime;
 #endif
 
 private:
   ~nsTimerEvent() { 
 #ifdef DEBUG
     if (mTimer)
       NS_WARNING("leaking reference to nsTimerImpl");
 #endif
+    MOZ_COUNT_DTOR(nsTimerEvent);
   }
 
   nsTimerImpl *mTimer;
   PRInt32      mGeneration;
 };
 
 NS_IMETHODIMP nsTimerEvent::Run()
 {
@@ -485,44 +487,50 @@ NS_IMETHODIMP nsTimerEvent::Run()
   }
 #endif
 
   timer->Fire();
 
   return NS_OK;
 }
 
-void nsTimerImpl::PostTimerEvent()
+nsresult nsTimerImpl::PostTimerEvent()
 {
   // XXX we may want to reuse this nsTimerEvent in the case of repeating timers.
 
   // Since TimerThread addref'd 'this' for us, we don't need to addref here.
   // We will release in destroyMyEvent.  We need to copy the generation number
   // from this timer into the event, so we can avoid firing a timer that was
   // re-initialized after being canceled.
 
-  nsTimerEvent* event = new nsTimerEvent(this, mGeneration);
+  nsRefPtr<nsTimerEvent> event = new nsTimerEvent(this, mGeneration);
   if (!event)
-    return;
+    return NS_ERROR_OUT_OF_MEMORY;
 
 #ifdef DEBUG_TIMERS
   if (PR_LOG_TEST(gTimerLog, PR_LOG_DEBUG)) {
     event->mInitTime = PR_IntervalNow();
   }
 #endif
 
   // If this is a repeating precise timer, we need to calculate the time for
   // the next timer to fire before we make the callback.
   if (mType == TYPE_REPEATING_PRECISE) {
     SetDelayInternal(mDelay);
-    if (gThread)
-      gThread->AddTimer(this);
+    if (gThread) {
+      nsresult rv = gThread->AddTimer(this);
+      if (NS_FAILED(rv))
+        return rv;
+    }
   }
 
-  mCallingThread->Dispatch(event, NS_DISPATCH_NORMAL);
+  nsresult rv = mCallingThread->Dispatch(event, NS_DISPATCH_NORMAL);
+  if (NS_FAILED(rv) && gThread)
+    gThread->RemoveTimer(this);
+  return rv;
 }
 
 void nsTimerImpl::SetDelayInternal(PRUint32 aDelay)
 {
   PRIntervalTime delayInterval = PR_MillisecondsToInterval(aDelay);
   if (delayInterval > DELAY_INTERVAL_MAX) {
     delayInterval = DELAY_INTERVAL_MAX;
     aDelay = PR_IntervalToMilliseconds(delayInterval);
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -39,17 +39,16 @@
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef nsTimerImpl_h___
 #define nsTimerImpl_h___
 
 //#define FORCE_PR_LOG /* Allow logging in the release build */
 
 #include "nsITimer.h"
-#include "nsVoidArray.h"
 #include "nsIThread.h"
 #include "nsIObserver.h"
 
 #include "nsCOMPtr.h"
 
 #include "prlog.h"
 
 #if defined(PR_LOGGING)
@@ -91,17 +90,17 @@ public:
   nsTimerImpl();
 
   static NS_HIDDEN_(nsresult) Startup();
   static NS_HIDDEN_(void) Shutdown();
 
   friend class TimerThread;
 
   void Fire();
-  void PostTimerEvent();
+  nsresult PostTimerEvent();
   void SetDelayInternal(PRUint32 aDelay);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSITIMER
 
   PRInt32 GetGeneration() { return mGeneration; }
 
 private: