Bug 330128. Calling cancel() on a timer doesn't drop ref to the callback. r=brendan, sr=bzbarsky, a=beltzner

xpcom/threads/nsTimerImpl.cpp

@@ -285,6 +285,8 @@ NS_IMETHODIMP nsTimerImpl::Cancel()
   if (gThread)
     gThread->RemoveTimer(this);
 
+  ReleaseCallback();
+
   return NS_OK;
 }
 
@@ -378,22 +380,45 @@ void nsTimerImpl::Fire()
     gThread->UpdateFilter(mDelay, timeout, now);
 
   mFiring = PR_TRUE;
+  
+  // Handle callbacks that re-init the timer, but avoid leaking.
+  // See bug 330128.
+  CallbackUnion callback = mCallback;
+  PRUintn callbackType = mCallbackType;
+  if (callbackType == CALLBACK_TYPE_INTERFACE)
+    NS_ADDREF(callback.i);
+  else if (callbackType == CALLBACK_TYPE_OBSERVER)
+    NS_ADDREF(callback.o);
+  ReleaseCallback();
 
-  switch (mCallbackType) {
+  switch (callbackType) {
     case CALLBACK_TYPE_FUNC:
-      mCallback.c(this, mClosure);
+      callback.c(this, mClosure);
       break;
     case CALLBACK_TYPE_INTERFACE:
-      mCallback.i->Notify(this);
+      callback.i->Notify(this);
       break;
     case CALLBACK_TYPE_OBSERVER:
-      mCallback.o->Observe(static_cast<nsITimer*>(this),
+      callback.o->Observe(static_cast<nsITimer*>(this),
-                           NS_TIMER_CALLBACK_TOPIC,
+                          NS_TIMER_CALLBACK_TOPIC,
-                           nsnull);
+                          nsnull);
       break;
     default:;
   }
 
+  // If the callback didn't re-init the timer, and it's not a one-shot timer,
+  // restore the callback state.
+  if (mCallbackType == CALLBACK_TYPE_UNKNOWN && callbackType != TYPE_ONE_SHOT) {
+    mCallback = callback;
+    mCallbackType = callbackType;
+  } else {
+    // The timer was a one-shot, or the callback was reinitialized.
+    if (callbackType == CALLBACK_TYPE_INTERFACE)
+      NS_RELEASE(callback.i);
+    else if (callbackType == CALLBACK_TYPE_OBSERVER)
+      NS_RELEASE(callback.o);
+  }
+
   mFiring = PR_FALSE;
 
 #ifdef DEBUG_TIMERS

xpcom/threads/nsTimerImpl.h

@@ -114,13 +114,14 @@ private:
       NS_RELEASE(mCallback.i);
     else if (mCallbackType == CALLBACK_TYPE_OBSERVER)
       NS_RELEASE(mCallback.o);
+    mCallbackType = CALLBACK_TYPE_UNKNOWN;
   }
 
   nsCOMPtr<nsIThread>   mCallingThread;
 
   void *                mClosure;
 
-  union {
+  union CallbackUnion {
     nsTimerCallbackFunc c;
     nsITimerCallback *  i;
     nsIObserver *       o;