Bug 744836 - Modify nsTimerEvent to hold its timer reference until the nsTimerEvent itself is destroyed. r=bsmedberg, r=ehsan, a=akeybl
☠☠ backed out by 5d2958ada047 ☠ ☠
authorAaron Klotz <aklotz@mozilla.com>
Thu, 30 May 2013 10:19:18 -0600
changeset 142795 dc00e08878b8bd1bb4f9a2d36a295d94758a4334
parent 142794 8ee3ad5ab3e14ba5a70c5b63b2ce7f29adb16296
child 142796 83d8259caa7b551d47a712fcf3f0e506a9d8c0e3
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg, ehsan, akeybl
bugs744836
milestone23.0a2
Bug 744836 - Modify nsTimerEvent to hold its timer reference until the nsTimerEvent itself is destroyed. r=bsmedberg, r=ehsan, a=akeybl
xpcom/threads/nsTimerImpl.cpp
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -101,17 +101,17 @@ public:
 
 } // anonymous namespace
 
 class nsTimerEvent : public nsRunnable {
 public:
   NS_IMETHOD Run();
 
   nsTimerEvent(nsTimerImpl *timer, int32_t generation)
-    : mTimer(timer), mGeneration(generation) {
+    : mTimer(dont_AddRef(timer)), mGeneration(generation) {
     // timer is already addref'd for us
     MOZ_COUNT_CTOR(nsTimerEvent);
 
     MOZ_ASSERT(gThread->IsOnTimerThread(),
                "nsTimer must always be allocated on the timer thread");
 
     PR_ATOMIC_INCREMENT(&sAllocatorUsers);
   }
@@ -130,28 +130,24 @@ public:
   void operator delete(void* p) {
     sAllocator->Free(p);
     DeleteAllocatorIfNeeded();
   }
 
 private:
   nsTimerEvent(); // Not implemented
   ~nsTimerEvent() {
-#ifdef DEBUG
-    if (mTimer)
-      NS_WARNING("leaking reference to nsTimerImpl");
-#endif
     MOZ_COUNT_DTOR(nsTimerEvent);
 
     MOZ_ASSERT(!sCanDeleteAllocator || sAllocatorUsers > 0,
                "This will result in us attempting to deallocate the nsTimerEvent allocator twice");
     PR_ATOMIC_DECREMENT(&sAllocatorUsers);
   }
 
-  nsTimerImpl *mTimer;
+  nsRefPtr<nsTimerImpl> mTimer;
   int32_t      mGeneration;
 
   static TimerEventAllocator* sAllocator;
   static int32_t sAllocatorUsers;
   static bool sCanDeleteAllocator;
 };
 
 TimerEventAllocator* nsTimerEvent::sAllocator = nullptr;
@@ -611,32 +607,29 @@ void nsTimerEvent::DeleteAllocatorIfNeed
   if (sCanDeleteAllocator && sAllocatorUsers == 0) {
     delete sAllocator;
     sAllocator = nullptr;
   }
 }
 
 NS_IMETHODIMP nsTimerEvent::Run()
 {
-  nsRefPtr<nsTimerImpl> timer;
-  timer.swap(mTimer);
-
-  if (mGeneration != timer->GetGeneration())
+  if (mGeneration != mTimer->GetGeneration())
     return NS_OK;
 
 #ifdef DEBUG_TIMERS
   if (PR_LOG_TEST(GetTimerLog(), PR_LOG_DEBUG)) {
     TimeStamp now = TimeStamp::Now();
     PR_LOG(GetTimerLog(), PR_LOG_DEBUG,
            ("[this=%p] time between PostTimerEvent() and Fire(): %fms\n",
             this, (now - mInitTime).ToMilliseconds()));
   }
 #endif
 
-  timer->Fire();
+  mTimer->Fire();
 
   return NS_OK;
 }
 
 nsresult nsTimerImpl::PostTimerEvent()
 {
   if (!mEventTarget) {
     NS_ERROR("Attempt to post timer event to NULL event target");