[tbb-commits] [tor-browser/esr24] Bug 1024765 - Part 2: Make refcounting logic around PostTimerEvent more explicit. r=bz, a=sledru
mikeperry at torproject.org
mikeperry at torproject.org
Fri Aug 29 05:26:42 UTC 2014
commit 24b9acce7908ed422611abb09b77ae5ae222ce0a
Author: Byron Campen [:bwc] <docfaraday at gmail.com>
Date: Mon Jun 16 14:13:04 2014 -0700
Bug 1024765 - Part 2: Make refcounting logic around PostTimerEvent more explicit. r=bz, a=sledru
---
xpcom/threads/TimerThread.cpp | 17 +++++++----
xpcom/threads/nsTimerImpl.cpp | 67 +++++++++++++++++++++++++++--------------
xpcom/threads/nsTimerImpl.h | 4 ++-
3 files changed, 59 insertions(+), 29 deletions(-)
diff --git a/xpcom/threads/TimerThread.cpp b/xpcom/threads/TimerThread.cpp
index f8a7cb7..ebf5751 100644
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -211,8 +211,9 @@ NS_IMETHODIMP TimerThread::Run()
// must be racing with us, blocked in gThread->RemoveTimer waiting
// for TimerThread::mMonitor, under nsTimerImpl::Release.
- NS_ADDREF(timer);
+ nsRefPtr<nsTimerImpl> timerRef(timer);
RemoveTimerInternal(timer);
+ timer = nullptr;
{
// We release mMonitor around the Fire call to avoid deadlock.
@@ -222,16 +223,21 @@ NS_IMETHODIMP TimerThread::Run()
if (PR_LOG_TEST(GetTimerLog(), PR_LOG_DEBUG)) {
PR_LOG(GetTimerLog(), PR_LOG_DEBUG,
("Timer thread woke up %fms from when it was supposed to\n",
- fabs((now - timer->mTimeout).ToMilliseconds())));
+ fabs((now - timerRef->mTimeout).ToMilliseconds())));
}
#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.
- if (NS_FAILED(timer->PostTimerEvent())) {
- nsrefcnt rc;
- NS_RELEASE2(timer, rc);
+ timerRef = nsTimerImpl::PostTimerEvent(timerRef.forget());
+
+ if (timerRef) {
+ // We got our reference back due to an error.
+ // Unhook the nsRefPtr, and release manually so we can get the
+ // refcount.
+ nsrefcnt rc = timerRef.forget().get()->Release();
+ (void)rc;
// The nsITimer interface requires that its users keep a reference
// to the timers they use while those timers are initialized but
@@ -246,7 +252,6 @@ NS_IMETHODIMP TimerThread::Run()
// preventing this situation from occurring.
MOZ_ASSERT(rc != 0, "destroyed timer off its target thread!");
}
- timer = nullptr;
}
if (mShutdown)
diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp
index fd4b391..717f280 100644
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -105,9 +105,10 @@ class nsTimerEvent : public nsRunnable {
public:
NS_IMETHOD Run();
- nsTimerEvent(nsTimerImpl *timer, int32_t generation)
- : mTimer(dont_AddRef(timer)), mGeneration(generation) {
- // timer is already addref'd for us
+ nsTimerEvent()
+ : mTimer()
+ , mGeneration(0)
+ {
MOZ_COUNT_CTOR(nsTimerEvent);
MOZ_ASSERT(gThread->IsOnTimerThread(),
@@ -132,8 +133,18 @@ public:
DeleteAllocatorIfNeeded();
}
+ already_AddRefed<nsTimerImpl> ForgetTimer()
+ {
+ return mTimer.forget();
+ }
+
+ void SetTimer(already_AddRefed<nsTimerImpl> aTimer)
+ {
+ mTimer = aTimer;
+ mGeneration = mTimer->GetGeneration();
+ }
+
private:
- nsTimerEvent(); // Not implemented
~nsTimerEvent() {
MOZ_COUNT_DTOR(nsTimerEvent);
@@ -629,23 +640,27 @@ NS_IMETHODIMP nsTimerEvent::Run()
return NS_OK;
}
-nsresult nsTimerImpl::PostTimerEvent()
+already_AddRefed<nsTimerImpl> nsTimerImpl::PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef)
{
- if (!mEventTarget) {
+ nsRefPtr<nsTimerImpl> timer(aTimerRef);
+ if (!timer->mEventTarget) {
NS_ERROR("Attempt to post timer event to NULL event target");
- return NS_ERROR_NOT_INITIALIZED;
+ return timer.forget();
}
// 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.
+ // Since TimerThread addref'd 'timer' for us, we don't need to addref here.
+ // We will release either in ~nsTimerEvent(), or pass the reference back to
+ // the caller. 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.
- nsRefPtr<nsTimerEvent> event = new nsTimerEvent(this, mGeneration);
+ // Note: We override operator new for this class, and the override is
+ // fallible!
+ nsRefPtr<nsTimerEvent> event = new nsTimerEvent;
if (!event)
- return NS_ERROR_OUT_OF_MEMORY;
+ return timer.forget();
#ifdef DEBUG_TIMERS
if (PR_LOG_TEST(GetTimerLog(), PR_LOG_DEBUG)) {
@@ -655,21 +670,29 @@ nsresult nsTimerImpl::PostTimerEvent()
// 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 (IsRepeatingPrecisely()) {
- SetDelayInternal(mDelay);
+ if (timer->IsRepeatingPrecisely()) {
+ timer->SetDelayInternal(timer->mDelay);
// But only re-arm REPEATING_PRECISE timers.
- if (gThread && mType == TYPE_REPEATING_PRECISE) {
- nsresult rv = gThread->AddTimer(this);
+ if (gThread && timer->mType == TYPE_REPEATING_PRECISE) {
+ nsresult rv = gThread->AddTimer(timer);
if (NS_FAILED(rv))
- return rv;
+ return timer.forget();
}
}
- nsresult rv = mEventTarget->Dispatch(event, NS_DISPATCH_NORMAL);
- if (NS_FAILED(rv) && gThread)
- gThread->RemoveTimer(this);
- return rv;
+ nsIEventTarget* target = timer->mEventTarget;
+ event->SetTimer(timer.forget());
+
+ nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
+ if (NS_FAILED(rv)) {
+ timer = event->ForgetTimer();
+ if (gThread)
+ gThread->RemoveTimer(timer);
+ }
+ return timer.forget();
+
+ return nullptr;
}
void nsTimerImpl::SetDelayInternal(uint32_t aDelay)
diff --git a/xpcom/threads/nsTimerImpl.h b/xpcom/threads/nsTimerImpl.h
index c318cf1..4a24060 100644
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -54,7 +54,9 @@ public:
friend struct TimerAdditionComparator;
void Fire();
- nsresult PostTimerEvent();
+ // If a failure is encountered, the reference is returned to the caller
+ static already_AddRefed<nsTimerImpl> PostTimerEvent(
+ already_AddRefed<nsTimerImpl> aTimerRef);
void SetDelayInternal(uint32_t aDelay);
NS_DECL_ISUPPORTS
More information about the tbb-commits
mailing list