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
--- 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: