diff options
author | Moonchild <moonchild@palemoon.org> | 2021-12-08 02:33:23 +0000 |
---|---|---|
committer | Moonchild <moonchild@palemoon.org> | 2021-12-08 02:33:23 +0000 |
commit | c7b40616bdf867a03192d013ea1f664b28580e47 (patch) | |
tree | f22123359f885198e9e7ee91a5621f81ef91dc8d /xpcom | |
parent | d855d7e1cfc97017ae288d2c631b1516bca157e3 (diff) | |
download | aura-central-c7b40616bdf867a03192d013ea1f664b28580e47.tar.gz |
[XPCOM] Simplify nsITimer API.
It's much simpler to hold a death grip on the timer than to have a fragile
"release upon cancel" construct.
Diffstat (limited to 'xpcom')
-rw-r--r-- | xpcom/threads/TimerThread.cpp | 15 | ||||
-rw-r--r-- | xpcom/threads/nsTimerImpl.cpp | 14 |
2 files changed, 12 insertions, 17 deletions
diff --git a/xpcom/threads/TimerThread.cpp b/xpcom/threads/TimerThread.cpp index db7c92462..36168e373 100644 --- a/xpcom/threads/TimerThread.cpp +++ b/xpcom/threads/TimerThread.cpp @@ -140,11 +140,7 @@ public: nsresult Cancel() override { - // Since nsTimerImpl is not thread-safe, we should release |mTimer| - // here in the target thread to avoid race condition. Otherwise, - // ~nsTimerEvent() which calls nsTimerImpl::Release() could run in the - // timer thread and result in race condition. - mTimer = nullptr; + mTimer->Cancel(); return NS_OK; } @@ -269,11 +265,6 @@ nsTimerEvent::DeleteAllocatorIfNeeded() NS_IMETHODIMP nsTimerEvent::Run() { - if (!mTimer) { - MOZ_ASSERT(false); - return NS_OK; - } - if (MOZ_LOG_TEST(GetTimerLog(), LogLevel::Debug)) { TimeStamp now = TimeStamp::Now(); MOZ_LOG(GetTimerLog(), LogLevel::Debug, @@ -283,9 +274,7 @@ nsTimerEvent::Run() mTimer->Fire(mGeneration); - // We call Cancel() to correctly release mTimer. - // Read more in the Cancel() implementation. - return Cancel(); + return NS_OK; } nsresult diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp index 830315cef..9e58ed023 100644 --- a/xpcom/threads/nsTimerImpl.cpp +++ b/xpcom/threads/nsTimerImpl.cpp @@ -433,11 +433,13 @@ nsTimerImpl::Fire(int32_t aGeneration) uint8_t oldType; uint32_t oldDelay; TimeStamp oldTimeout; + nsCOMPtr<nsITimer> timer; { + MutexAutoLock lock(mMutex); + // Don't fire callbacks or fiddle with refcounts when the mutex is locked. // If some other thread Cancels/Inits after this, they're just too late. - MutexAutoLock lock(mMutex); if (aGeneration != mGeneration) { return; } @@ -446,6 +448,10 @@ nsTimerImpl::Fire(int32_t aGeneration) oldType = mType; oldDelay = mDelay; oldTimeout = mTimeout; + // Ensure that the nsITimer does not unhook from the nsTimerImpl during + // Fire; this will cause null pointer crashes if the user of the timer drops + // its reference, and then uses the nsITimer* passed in the callback. + timer = mITimer; } PROFILER_LABEL("Timer", "Fire", @@ -475,13 +481,13 @@ nsTimerImpl::Fire(int32_t aGeneration) switch (mCallbackDuringFire.mType) { case Callback::Type::Function: - mCallbackDuringFire.mCallback.c(mITimer, mCallbackDuringFire.mClosure); + mCallbackDuringFire.mCallback.c(timer, mCallbackDuringFire.mClosure); break; case Callback::Type::Interface: - mCallbackDuringFire.mCallback.i->Notify(mITimer); + mCallbackDuringFire.mCallback.i->Notify(timer); break; case Callback::Type::Observer: - mCallbackDuringFire.mCallback.o->Observe(mITimer, NS_TIMER_CALLBACK_TOPIC, + mCallbackDuringFire.mCallback.o->Observe(timer, NS_TIMER_CALLBACK_TOPIC, nullptr); break; default: |