From 08e0f3cf36d0dd2e60fa35b08b482c10186294d3 Mon Sep 17 00:00:00 2001 From: Alexandru Michis Date: Tue, 18 May 2021 20:25:56 +0300 Subject: [PATCH] Backed out 4 changesets (bug 1711090) for causing bustages in nsTimerImpl.cpp CLOSED TREE Backed out changeset 5c6f0950714d (bug 1711090) Backed out changeset 0b6a886eea8a (bug 1711090) Backed out changeset fc9c788ff41d (bug 1711090) Backed out changeset ecc51d9ad027 (bug 1711090) --- docshell/base/nsDSURIContentListener.cpp | 1 - dom/base/EventSource.cpp | 3 +- dom/media/mediacontrol/MediaController.cpp | 5 +- .../ServiceWorkerShutdownBlocker.cpp | 3 +- netwerk/dns/TRR.cpp | 2 +- netwerk/protocol/http/nsHttpTransaction.cpp | 2 +- netwerk/system/mac/nsNetworkLinkService.mm | 2 +- .../ApplicationReputation.cpp | 2 +- xpcom/tests/gtest/TestTimers.cpp | 31 -- xpcom/threads/nsITimer.idl | 80 ++-- xpcom/threads/nsTimerImpl.cpp | 369 +++++++++++------- xpcom/threads/nsTimerImpl.h | 109 ++++-- 12 files changed, 372 insertions(+), 237 deletions(-) diff --git a/docshell/base/nsDSURIContentListener.cpp b/docshell/base/nsDSURIContentListener.cpp index a632e4f2c365..6af92f53e5a7 100644 --- a/docshell/base/nsDSURIContentListener.cpp +++ b/docshell/base/nsDSURIContentListener.cpp @@ -30,7 +30,6 @@ NS_IMPL_RELEASE(MaybeCloseWindowHelper) NS_INTERFACE_MAP_BEGIN(MaybeCloseWindowHelper) NS_INTERFACE_MAP_ENTRY(nsISupports) - NS_INTERFACE_MAP_ENTRY(nsITimerCallback) NS_INTERFACE_MAP_END MaybeCloseWindowHelper::MaybeCloseWindowHelper(BrowsingContext* aContentContext) diff --git a/dom/base/EventSource.cpp b/dom/base/EventSource.cpp index adee170d561d..3fbe12b40a67 100644 --- a/dom/base/EventSource.cpp +++ b/dom/base/EventSource.cpp @@ -361,8 +361,7 @@ class EventSourceImpl final : public nsIObserver, NS_IMPL_ISUPPORTS(EventSourceImpl, nsIObserver, nsIStreamListener, nsIRequestObserver, nsIChannelEventSink, nsIInterfaceRequestor, nsISupportsWeakReference, - nsIEventTarget, nsIThreadRetargetableStreamListener, - nsITimerCallback) + nsIEventTarget, nsIThreadRetargetableStreamListener) EventSourceImpl::EventSourceImpl(EventSource* aEventSource, nsICookieJarSettings* aCookieJarSettings) diff --git a/dom/media/mediacontrol/MediaController.cpp b/dom/media/mediacontrol/MediaController.cpp index b2b3c13ae85a..5d1e03692fdb 100644 --- a/dom/media/mediacontrol/MediaController.cpp +++ b/dom/media/mediacontrol/MediaController.cpp @@ -26,9 +26,8 @@ namespace mozilla::dom { NS_IMPL_CYCLE_COLLECTION_INHERITED(MediaController, DOMEventTargetHelper) -NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED(MediaController, - DOMEventTargetHelper, - nsITimerCallback) +NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(MediaController, + DOMEventTargetHelper) NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(MediaController, DOMEventTargetHelper) NS_IMPL_CYCLE_COLLECTION_TRACE_END diff --git a/dom/serviceworkers/ServiceWorkerShutdownBlocker.cpp b/dom/serviceworkers/ServiceWorkerShutdownBlocker.cpp index 662ad6b5461c..2aa19653ec6b 100644 --- a/dom/serviceworkers/ServiceWorkerShutdownBlocker.cpp +++ b/dom/serviceworkers/ServiceWorkerShutdownBlocker.cpp @@ -23,8 +23,7 @@ namespace mozilla { namespace dom { -NS_IMPL_ISUPPORTS(ServiceWorkerShutdownBlocker, nsIAsyncShutdownBlocker, - nsITimerCallback) +NS_IMPL_ISUPPORTS(ServiceWorkerShutdownBlocker, nsIAsyncShutdownBlocker) NS_IMETHODIMP ServiceWorkerShutdownBlocker::GetName(nsAString& aNameOut) { aNameOut = nsLiteralString( diff --git a/netwerk/dns/TRR.cpp b/netwerk/dns/TRR.cpp index 94a23ba5bc37..88c422265940 100644 --- a/netwerk/dns/TRR.cpp +++ b/netwerk/dns/TRR.cpp @@ -49,7 +49,7 @@ extern mozilla::LazyLogModule gHostResolverLog; MOZ_LOG_TEST(mozilla::net::gHostResolverLog, mozilla::LogLevel::Debug) NS_IMPL_ISUPPORTS(TRR, nsIHttpPushListener, nsIInterfaceRequestor, - nsIStreamListener, nsIRunnable, nsITimerCallback) + nsIStreamListener, nsIRunnable) // when firing off a normal A or AAAA query TRR::TRR(AHostResolver* aResolver, nsHostRecord* aRec, enum TrrType aType) diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index c8ea1b018ad4..d3034c0a435e 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -2844,7 +2844,7 @@ nsHttpTransaction::Release() { } NS_IMPL_QUERY_INTERFACE(nsHttpTransaction, nsIInputStreamCallback, - nsIOutputStreamCallback, nsITimerCallback) + nsIOutputStreamCallback) //----------------------------------------------------------------------------- // nsHttpTransaction::nsIInputStreamCallback diff --git a/netwerk/system/mac/nsNetworkLinkService.mm b/netwerk/system/mac/nsNetworkLinkService.mm index 09dd4650107a..5d478de1c139 100644 --- a/netwerk/system/mac/nsNetworkLinkService.mm +++ b/netwerk/system/mac/nsNetworkLinkService.mm @@ -85,7 +85,7 @@ static void CFReleaseSafe(CFTypeRef cf) { } } -NS_IMPL_ISUPPORTS(nsNetworkLinkService, nsINetworkLinkService, nsIObserver, nsITimerCallback) +NS_IMPL_ISUPPORTS(nsNetworkLinkService, nsINetworkLinkService, nsIObserver) nsNetworkLinkService::nsNetworkLinkService() : mLinkUp(true), diff --git a/toolkit/components/reputationservice/ApplicationReputation.cpp b/toolkit/components/reputationservice/ApplicationReputation.cpp index 3bd831dd2061..b90a6db94ecc 100644 --- a/toolkit/components/reputationservice/ApplicationReputation.cpp +++ b/toolkit/components/reputationservice/ApplicationReputation.cpp @@ -876,7 +876,7 @@ PendingDBLookup::HandleEvent(const nsACString& tables) { } NS_IMPL_ISUPPORTS(PendingLookup, nsIStreamListener, nsIRequestObserver, - nsIObserver, nsISupportsWeakReference, nsITimerCallback) + nsIObserver, nsISupportsWeakReference) PendingLookup::PendingLookup(nsIApplicationReputationQuery* aQuery, nsIApplicationReputationCallback* aCallback) diff --git a/xpcom/tests/gtest/TestTimers.cpp b/xpcom/tests/gtest/TestTimers.cpp index 2b2cb6b2158f..2940d3265a0b 100644 --- a/xpcom/tests/gtest/TestTimers.cpp +++ b/xpcom/tests/gtest/TestTimers.cpp @@ -632,34 +632,3 @@ TEST(Timers, FuzzTestTimers) } } } - -TEST(Timers, ClosureCallback) -{ - AutoCreateAndDestroyReentrantMonitor newMon; - ASSERT_TRUE(newMon); - - AutoTestThread testThread; - ASSERT_TRUE(testThread); - - nsIThread* notifiedThread = nullptr; - - nsCOMPtr timer; - nsresult rv = NS_NewTimerWithCallback( - getter_AddRefs(timer), - [&](nsITimer*) { - nsCOMPtr current(do_GetCurrentThread()); - - ReentrantMonitorAutoEnter mon(*newMon); - ASSERT_FALSE(notifiedThread); - notifiedThread = current; - mon.Notify(); - }, - 50, nsITimer::TYPE_ONE_SHOT, "(test) Timers.ClosureCallback", testThread); - ASSERT_TRUE(NS_SUCCEEDED(rv)); - - ReentrantMonitorAutoEnter mon(*newMon); - while (!notifiedThread) { - mon.Wait(); - } - ASSERT_EQ(notifiedThread, testThread); -} diff --git a/xpcom/threads/nsITimer.idl b/xpcom/threads/nsITimer.idl index 37322e5dbed6..96ea2943a51a 100644 --- a/xpcom/threads/nsITimer.idl +++ b/xpcom/threads/nsITimer.idl @@ -11,7 +11,6 @@ interface nsIEventTarget; %{C++ #include "mozilla/MemoryReporting.h" #include "mozilla/TimeStamp.h" -#include /** * The signature of the timer callback function passed to initWithFuncCallback. @@ -23,9 +22,27 @@ interface nsIEventTarget; */ class nsITimer; typedef void (*nsTimerCallbackFunc) (nsITimer *aTimer, void *aClosure); + +/** + * The signature of the timer name callback function passed to + * initWithNameableFuncCallback. + * This is the function that will get called when timer profiling is enabled + * via the "TimerFirings" log module. + * + * @param aTimer the timer which has expired + * @param aAnonymize whether the name should avoid including privacy sensitive info + * @param aClosure opaque parameter passed to initWithFuncCallback + * @param aBuf a buffer in which to put the name + * @param aLen the length of the buffer + */ +typedef void (*nsTimerNameCallbackFunc) (nsITimer *aTimer, + bool aAnonymize, + void *aClosure, + char *aBuf, size_t aLen); %} native nsTimerCallbackFunc(nsTimerCallbackFunc); +native nsTimerNameCallbackFunc(nsTimerNameCallbackFunc); [ref] native TimeDuration(mozilla::TimeDuration); /** @@ -184,6 +201,23 @@ interface nsITimer : nsISupports in unsigned long aType, in string aName); + /** + * Like initWithNamedFuncCallback, but instead of a timer name it takes a + * callback that will provide a name when the timer fires. + * + * @param aFunc The function to invoke + * @param aClosure An opaque pointer to pass to that function + * @param aDelay The millisecond interval + * @param aType Timer type per TYPE* consts defined above + * @param aNameCallback The callback function + */ + [noscript] void initWithNameableFuncCallback( + in nsTimerCallbackFunc aCallback, + in voidPtr aClosure, + in unsigned long aDelay, + in unsigned long aType, + in nsTimerNameCallbackFunc aNameCallback); + /** * The millisecond delay of the timeout. * @@ -270,34 +304,6 @@ NS_NewTimerWithCallback(nsITimerCallback* aCallback, uint32_t aType, nsIEventTarget* aTarget = nullptr); -nsresult -NS_NewTimerWithCallback(nsITimer** aTimer, - std::function&& aCallback, - uint32_t aDelay, - uint32_t aType, - const char* aNameString, - nsIEventTarget* aTarget = nullptr); -mozilla::Result, nsresult> -NS_NewTimerWithCallback(std::function&& aCallback, - uint32_t aDelay, - uint32_t aType, - const char* aNameString, - nsIEventTarget* aTarget = nullptr); - -nsresult -NS_NewTimerWithCallback(nsITimer** aTimer, - std::function&& aCallback, - const mozilla::TimeDuration& aDelay, - uint32_t aType, - const char* aNameString, - nsIEventTarget* aTarget = nullptr); -mozilla::Result, nsresult> -NS_NewTimerWithCallback(std::function&& aCallback, - const mozilla::TimeDuration& aDelay, - uint32_t aType, - const char* aNameString, - nsIEventTarget* aTarget = nullptr); - nsresult NS_NewTimerWithFuncCallback(nsITimer** aTimer, nsTimerCallbackFunc aCallback, @@ -314,6 +320,22 @@ NS_NewTimerWithFuncCallback(nsTimerCallbackFunc aCallback, const char* aNameString, nsIEventTarget* aTarget = nullptr); +nsresult +NS_NewTimerWithFuncCallback(nsITimer** aTimer, + nsTimerCallbackFunc aCallback, + void* aClosure, + uint32_t aDelay, + uint32_t aType, + nsTimerNameCallbackFunc aNameCallback, + nsIEventTarget* aTarget = nullptr); +mozilla::Result, nsresult> +NS_NewTimerWithFuncCallback(nsTimerCallbackFunc aCallback, + void* aClosure, + uint32_t aDelay, + uint32_t aType, + nsTimerNameCallbackFunc aNameCallback, + nsIEventTarget* aTarget = nullptr); + #define NS_TIMER_CONTRACTID "@mozilla.org/timer;1" #define NS_TIMER_CALLBACK_TOPIC "timer-callback" %} diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp index 7685326f76d2..9b4db3ffab55 100644 --- a/xpcom/threads/nsTimerImpl.cpp +++ b/xpcom/threads/nsTimerImpl.cpp @@ -114,45 +114,6 @@ nsresult NS_NewTimerWithCallback(nsITimer** aTimer, nsITimerCallback* aCallback, return NS_OK; } -mozilla::Result, nsresult> NS_NewTimerWithCallback( - std::function&& aCallback, uint32_t aDelay, uint32_t aType, - const char* aNameString, nsIEventTarget* aTarget) { - nsCOMPtr timer; - MOZ_TRY(NS_NewTimerWithCallback(getter_AddRefs(timer), std::move(aCallback), - aDelay, aType, aNameString, aTarget)); - return timer; -} -nsresult NS_NewTimerWithCallback(nsITimer** aTimer, - std::function&& aCallback, - uint32_t aDelay, uint32_t aType, - const char* aNameString, - nsIEventTarget* aTarget) { - return NS_NewTimerWithCallback(aTimer, std::move(aCallback), - TimeDuration::FromMilliseconds(aDelay), aType, - aNameString, aTarget); -} - -mozilla::Result, nsresult> NS_NewTimerWithCallback( - std::function&& aCallback, const TimeDuration& aDelay, - uint32_t aType, const char* aNameString, nsIEventTarget* aTarget) { - nsCOMPtr timer; - MOZ_TRY(NS_NewTimerWithCallback(getter_AddRefs(timer), std::move(aCallback), - aDelay, aType, aNameString, aTarget)); - return timer; -} -nsresult NS_NewTimerWithCallback(nsITimer** aTimer, - std::function&& aCallback, - const TimeDuration& aDelay, uint32_t aType, - const char* aNameString, - nsIEventTarget* aTarget) { - RefPtr timer = nsTimer::WithEventTarget(aTarget); - - MOZ_TRY(timer->InitWithClosureCallback(std::move(aCallback), aDelay, aType, - aNameString)); - timer.forget(aTimer); - return NS_OK; -} - mozilla::Result, nsresult> NS_NewTimerWithFuncCallback( nsTimerCallbackFunc aCallback, void* aClosure, uint32_t aDelay, uint32_t aType, const char* aNameString, nsIEventTarget* aTarget) { @@ -175,13 +136,50 @@ nsresult NS_NewTimerWithFuncCallback(nsITimer** aTimer, return NS_OK; } +mozilla::Result, nsresult> NS_NewTimerWithFuncCallback( + nsTimerCallbackFunc aCallback, void* aClosure, uint32_t aDelay, + uint32_t aType, nsTimerNameCallbackFunc aNameCallback, + nsIEventTarget* aTarget) { + nsCOMPtr timer; + MOZ_TRY(NS_NewTimerWithFuncCallback(getter_AddRefs(timer), aCallback, + aClosure, aDelay, aType, aNameCallback, + aTarget)); + return std::move(timer); +} +nsresult NS_NewTimerWithFuncCallback(nsITimer** aTimer, + nsTimerCallbackFunc aCallback, + void* aClosure, uint32_t aDelay, + uint32_t aType, + nsTimerNameCallbackFunc aNameCallback, + nsIEventTarget* aTarget) { + auto timer = nsTimer::WithEventTarget(aTarget); + + MOZ_TRY(timer->InitWithNameableFuncCallback(aCallback, aClosure, aDelay, + aType, aNameCallback)); + timer.forget(aTimer); + return NS_OK; +} + // This module prints info about which timers are firing, which is useful for // wakeups for the purposes of power profiling. Set the following environment // variable before starting the browser. // // MOZ_LOG=TimerFirings:4 // -// Then a line will be printed for every timer that fires. +// Then a line will be printed for every timer that fires. The name used for a +// |Callback::Type::Function| timer depends on the circumstances. +// +// - If it was explicitly named (e.g. it was initialized with +// InitWithNamedFuncCallback()) then that explicit name will be shown. +// +// - Otherwise, if we are on a platform that supports function name lookup +// (Mac or Linux) then the looked-up name will be shown with a +// "[from dladdr]" annotation. On Mac the looked-up name will be immediately +// useful. On Linux it'll need post-processing with `tools/rb/fix_stacks.py`. +// +// - Otherwise, no name will be printed. If many timers hit this case then +// you'll need to re-run the workload on a Mac to find out which timers they +// are, and then give them explicit names. // // If you redirect this output to a file called "out", you can then // post-process it with a command something like the following. @@ -248,7 +246,6 @@ nsTimerImpl::nsTimerImpl(nsITimer* aTimer, nsIEventTarget* aTarget) mGeneration(0), mITimer(aTimer), mMutex("nsTimerImpl::mMutex"), - mCallback(UnknownCallback{}), mFiring(0) { // XXX some code creates timers during xpcom shutdown, when threads are no // longer available, so we cannot turn this on yet. @@ -311,9 +308,7 @@ nsresult nsTimerImpl::InitCommon(const TimeDuration& aDelay, uint32_t aType, } gThread->RemoveTimer(this); - // If we have an existing callback, using `swap` ensures it's destroyed after - // the mutex is unlocked in our caller. - std::swap(mCallback, newCallback); + mCallback.swap(newCallback); ++mGeneration; mType = (uint8_t)aType; @@ -323,20 +318,40 @@ nsresult nsTimerImpl::InitCommon(const TimeDuration& aDelay, uint32_t aType, return gThread->AddTimer(this); } -nsresult nsTimerImpl::InitWithNamedFuncCallback(nsTimerCallbackFunc aFunc, - void* aClosure, uint32_t aDelay, - uint32_t aType, - const char* aName) { +nsresult nsTimerImpl::InitWithFuncCallbackCommon(nsTimerCallbackFunc aFunc, + void* aClosure, + uint32_t aDelay, + uint32_t aType, + const Callback::Name& aName) { if (NS_WARN_IF(!aFunc)) { return NS_ERROR_INVALID_ARG; } - Callback cb(FuncCallback{aFunc, aClosure, aName}); + Callback cb; // Goes out of scope after the unlock, prevents deadlock + cb.mType = Callback::Type::Function; + cb.mCallback.c = aFunc; + cb.mClosure = aClosure; + cb.mName = aName; MutexAutoLock lock(mMutex); return InitCommon(aDelay, aType, std::move(cb)); } +nsresult nsTimerImpl::InitWithNamedFuncCallback(nsTimerCallbackFunc aFunc, + void* aClosure, uint32_t aDelay, + uint32_t aType, + const char* aNameString) { + Callback::Name name(aNameString); + return InitWithFuncCallbackCommon(aFunc, aClosure, aDelay, aType, name); +} + +nsresult nsTimerImpl::InitWithNameableFuncCallback( + nsTimerCallbackFunc aFunc, void* aClosure, uint32_t aDelay, uint32_t aType, + nsTimerNameCallbackFunc aNameFunc) { + Callback::Name name(aNameFunc); + return InitWithFuncCallbackCommon(aFunc, aClosure, aDelay, aType, name); +} + nsresult nsTimerImpl::InitWithCallback(nsITimerCallback* aCallback, uint32_t aDelayInMs, uint32_t aType) { return InitHighResolutionWithCallback( @@ -349,8 +364,10 @@ nsresult nsTimerImpl::InitHighResolutionWithCallback( return NS_ERROR_INVALID_ARG; } - // Goes out of scope after the unlock, prevents deadlock - Callback cb(nsCOMPtr{aCallback}); + Callback cb; // Goes out of scope after the unlock, prevents deadlock + cb.mType = Callback::Type::Interface; + cb.mCallback.i = aCallback; + NS_ADDREF(cb.mCallback.i); MutexAutoLock lock(mMutex); return InitCommon(aDelay, aType, std::move(cb)); @@ -362,32 +379,22 @@ nsresult nsTimerImpl::Init(nsIObserver* aObserver, uint32_t aDelayInMs, return NS_ERROR_INVALID_ARG; } - Callback cb(nsCOMPtr{aObserver}); + Callback cb; // Goes out of scope after the unlock, prevents deadlock + cb.mType = Callback::Type::Observer; + cb.mCallback.o = aObserver; + NS_ADDREF(cb.mCallback.o); MutexAutoLock lock(mMutex); return InitCommon(aDelayInMs, aType, std::move(cb)); } -nsresult nsTimerImpl::InitWithClosureCallback( - std::function&& aCallback, const TimeDuration& aDelay, - uint32_t aType, const char* aNameString) { - if (NS_WARN_IF(!aCallback)) { - return NS_ERROR_INVALID_ARG; - } - - Callback cb(ClosureCallback{std::move(aCallback), aNameString}); - - MutexAutoLock lock(mMutex); - return InitCommon(aDelay, aType, std::move(cb)); -} - nsresult nsTimerImpl::Cancel() { CancelImpl(false); return NS_OK; } void nsTimerImpl::CancelImpl(bool aClearITimer) { - Callback cbTrash(UnknownCallback{}); + Callback cbTrash; RefPtr timerTrash; { @@ -396,9 +403,7 @@ void nsTimerImpl::CancelImpl(bool aClearITimer) { gThread->RemoveTimer(this); } - // The swap ensures our callback isn't dropped until after the mutex is - // unlocked. - std::swap(cbTrash, mCallback); + cbTrash.swap(mCallback); ++mGeneration; // Don't clear this if we're firing; once Fire returns, we'll get this call @@ -415,7 +420,7 @@ void nsTimerImpl::CancelImpl(bool aClearITimer) { nsresult nsTimerImpl::SetDelay(uint32_t aDelay) { MutexAutoLock lock(mMutex); - if (GetCallback().is() && !IsRepeating()) { + if (GetCallback().mType == Callback::Type::Unknown && !IsRepeating()) { // This may happen if someone tries to re-use a one-shot timer // by re-setting delay instead of reinitializing the timer. NS_ERROR( @@ -462,21 +467,18 @@ nsresult nsTimerImpl::GetType(uint32_t* aType) { nsresult nsTimerImpl::GetClosure(void** aClosure) { MutexAutoLock lock(mMutex); - if (GetCallback().is()) { - *aClosure = GetCallback().as().mClosure; - } else { - *aClosure = nullptr; - } + *aClosure = GetCallback().mClosure; return NS_OK; } nsresult nsTimerImpl::GetCallback(nsITimerCallback** aCallback) { MutexAutoLock lock(mMutex); - if (GetCallback().is()) { - NS_IF_ADDREF(*aCallback = GetCallback().as()); + if (GetCallback().mType == Callback::Type::Interface) { + NS_IF_ADDREF(*aCallback = GetCallback().mCallback.i); } else { *aCallback = nullptr; } + return NS_OK; } @@ -488,7 +490,7 @@ nsresult nsTimerImpl::GetTarget(nsIEventTarget** aTarget) { nsresult nsTimerImpl::SetTarget(nsIEventTarget* aTarget) { MutexAutoLock lock(mMutex); - if (NS_WARN_IF(!mCallback.is())) { + if (NS_WARN_IF(mCallback.mType != Callback::Type::Unknown)) { return NS_ERROR_ALREADY_INITIALIZED; } @@ -509,7 +511,7 @@ void nsTimerImpl::Fire(int32_t aGeneration) { uint8_t oldType; uint32_t oldDelay; TimeStamp oldTimeout; - Callback callbackDuringFire(UnknownCallback{}); + Callback callbackDuringFire; nsCOMPtr kungFuDeathGrip; { @@ -555,14 +557,19 @@ void nsTimerImpl::Fire(int32_t aGeneration) { LogFiring(callbackDuringFire, oldType, oldDelay); } - callbackDuringFire.match( - [](const UnknownCallback&) {}, - [&](const InterfaceCallback& i) { i->Notify(mITimer); }, - [&](const ObserverCallback& o) { - o->Observe(mITimer, NS_TIMER_CALLBACK_TOPIC, nullptr); - }, - [&](const FuncCallback& f) { f.mFunc(mITimer, f.mClosure); }, - [&](const ClosureCallback& c) { c.mFunc(mITimer); }); + switch (callbackDuringFire.mType) { + case Callback::Type::Function: + callbackDuringFire.mCallback.c(mITimer, callbackDuringFire.mClosure); + break; + case Callback::Type::Interface: + callbackDuringFire.mCallback.i->Notify(mITimer); + break; + case Callback::Type::Observer: + callbackDuringFire.mCallback.o->Observe(mITimer, NS_TIMER_CALLBACK_TOPIC, + nullptr); + break; + default:; + } MutexAutoLock lock(mMutex); if (aGeneration == mGeneration) { @@ -578,9 +585,7 @@ void nsTimerImpl::Fire(int32_t aGeneration) { } } else { // Non-repeating timer that has not been re-scheduled. Clear. - // XXX(nika): Other callsites seem to go to some effort to avoid - // destroying mCallback when it's held. Why not this one? - mCallback = mozilla::AsVariant(UnknownCallback{}); + mCallback.clear(); } } @@ -591,6 +596,15 @@ void nsTimerImpl::Fire(int32_t aGeneration) { (TimeStamp::Now() - now).ToMilliseconds())); } +#if defined(HAVE_DLADDR) && defined(HAVE___CXA_DEMANGLE) +# define USE_DLADDR 1 +#endif + +#ifdef USE_DLADDR +# include +# include +#endif + // See the big comment above GetTimerFiringsLog() to understand this code. void nsTimerImpl::LogFiring(const Callback& aCallback, uint8_t aType, uint32_t aDelay) { @@ -616,54 +630,138 @@ void nsTimerImpl::LogFiring(const Callback& aCallback, uint8_t aType, MOZ_CRASH("bad type"); } - aCallback.match( - [&](const UnknownCallback&) { - MOZ_LOG( - GetTimerFiringsLog(), LogLevel::Debug, - ("[%d] ??? timer (%s, %5d ms)\n", getpid(), typeStr, aDelay)); - }, - [&](const InterfaceCallback& i) { - MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, - ("[%d] iface timer (%s %5d ms): %p\n", getpid(), typeStr, - aDelay, i.get())); - }, - [&](const ObserverCallback& o) { - MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, - ("[%d] obs timer (%s %5d ms): %p\n", getpid(), typeStr, - aDelay, o.get())); - }, - [&](const FuncCallback& f) { - MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, - ("[%d] fn timer (%s %5d ms): %s\n", getpid(), typeStr, - aDelay, f.mName)); - }, - [&](const ClosureCallback& c) { - MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, - ("[%d] closure timer (%s %5d ms): %s\n", getpid(), typeStr, - aDelay, c.mName)); - }); + switch (aCallback.mType) { + case Callback::Type::Function: { + bool needToFreeName = false; + const char* annotation = ""; + const char* name; + static const size_t buflen = 1024; + char buf[buflen]; + + if (aCallback.mName.is()) { + name = aCallback.mName.as(); + + } else if (aCallback.mName.is()) { + aCallback.mName.as()( + mITimer, /* aAnonymize = */ false, aCallback.mClosure, buf, buflen); + name = buf; + + } else { + MOZ_ASSERT(aCallback.mName.is()); +#ifdef USE_DLADDR + annotation = "[from dladdr] "; + + Dl_info info; + void* addr = reinterpret_cast(aCallback.mCallback.c); + if (dladdr(addr, &info) == 0) { + name = "???[dladdr: failed]"; + + } else if (info.dli_sname) { + int status; + name = abi::__cxa_demangle(info.dli_sname, nullptr, nullptr, &status); + if (status == 0) { + // Success. Because we didn't pass in a buffer to __cxa_demangle it + // allocates its own one with malloc() which we must free() later. + MOZ_ASSERT(name); + needToFreeName = true; + } else if (status == -1) { + name = "???[__cxa_demangle: OOM]"; + } else if (status == -2) { + name = "???[__cxa_demangle: invalid mangled name]"; + } else if (status == -3) { + name = "???[__cxa_demangle: invalid argument]"; + } else { + name = "???[__cxa_demangle: unexpected status value]"; + } + + } else if (info.dli_fname) { + // The "#0: " prefix is necessary for `fix_stacks.py` to interpret + // this string as something to convert. + SprintfLiteral(buf, "#0: ???[%s +0x%" PRIxPTR "]\n", info.dli_fname, + uintptr_t(addr) - uintptr_t(info.dli_fbase)); + name = buf; + + } else { + name = "???[dladdr: no symbol or shared object obtained]"; + } +#else + name = "???[dladdr is unimplemented or doesn't work well on this OS]"; +#endif + } + + MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, + ("[%d] fn timer (%s %5d ms): %s%s\n", getpid(), typeStr, + aDelay, annotation, name)); + + if (needToFreeName) { + free(const_cast(name)); + } + + break; + } + + case Callback::Type::Interface: { + MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, + ("[%d] iface timer (%s %5d ms): %p\n", getpid(), typeStr, aDelay, + aCallback.mCallback.i)); + break; + } + + case Callback::Type::Observer: { + MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, + ("[%d] obs timer (%s %5d ms): %p\n", getpid(), typeStr, aDelay, + aCallback.mCallback.o)); + break; + } + + case Callback::Type::Unknown: + default: { + MOZ_LOG(GetTimerFiringsLog(), LogLevel::Debug, + ("[%d] ??? timer (%s, %5d ms)\n", getpid(), typeStr, aDelay)); + break; + } + } } void nsTimerImpl::GetName(nsACString& aName) { MutexAutoLock lock(mMutex); - GetCallback().match( - [&](const UnknownCallback&) { aName.AssignLiteral("Canceled_timer"); }, - [&](const InterfaceCallback& i) { - if (nsCOMPtr named = do_QueryInterface(i)) { - named->GetName(aName); - } else { - aName.AssignLiteral("Anonymous_interface_timer"); - } - }, - [&](const ObserverCallback& o) { - if (nsCOMPtr named = do_QueryInterface(o)) { - named->GetName(aName); - } else { - aName.AssignLiteral("Anonymous_observer_timer"); - } - }, - [&](const FuncCallback& f) { aName.Assign(f.mName); }, - [&](const ClosureCallback& c) { aName.Assign(c.mName); }); + Callback& cb(GetCallback()); + switch (cb.mType) { + case Callback::Type::Function: + if (cb.mName.is()) { + aName.Assign(cb.mName.as()); + } else if (cb.mName.is()) { + static const size_t buflen = 1024; + char buf[buflen]; + cb.mName.as()(mITimer, /* aAnonymize = */ true, + cb.mClosure, buf, buflen); + aName.Assign(buf); + } else { + MOZ_ASSERT(cb.mName.is()); + aName.AssignLiteral("Anonymous_callback_timer"); + } + break; + + case Callback::Type::Interface: + if (nsCOMPtr named = do_QueryInterface(cb.mCallback.i)) { + named->GetName(aName); + } else { + aName.AssignLiteral("Anonymous_interface_timer"); + } + break; + + case Callback::Type::Observer: + if (nsCOMPtr named = do_QueryInterface(cb.mCallback.o)) { + named->GetName(aName); + } else { + aName.AssignLiteral("Anonymous_observer_timer"); + } + break; + + case Callback::Type::Unknown: + aName.AssignLiteral("Canceled_timer"); + break; + } } void nsTimerImpl::SetHolder(nsTimerImplHolder* aHolder) { mHolder = aHolder; } @@ -695,6 +793,9 @@ nsresult nsTimer::XPCOMConstructor(nsISupports* aOuter, REFNSIID aIID, return timer->QueryInterface(aIID, aResult); } +/* static */ +const nsTimerImpl::Callback::NameNothing nsTimerImpl::Callback::Nothing = 0; + #ifdef MOZ_TASK_TRACER void nsTimerImpl::GetTLSTraceInfo() { mTracedTask.GetTLSTraceInfo(); } diff --git a/xpcom/threads/nsTimerImpl.h b/xpcom/threads/nsTimerImpl.h index eea146bd49c9..61d4d6d0b2af 100644 --- a/xpcom/threads/nsTimerImpl.h +++ b/xpcom/threads/nsTimerImpl.h @@ -67,30 +67,87 @@ class nsTimerImpl { int32_t GetGeneration() { return mGeneration; } - struct UnknownCallback {}; + struct Callback { + Callback() : mType(Type::Unknown), mName(Nothing), mClosure(nullptr) { + mCallback.c = nullptr; + } - using InterfaceCallback = nsCOMPtr; + Callback(const Callback& other) : Callback() { *this = other; } - using ObserverCallback = nsCOMPtr; + enum class Type : uint8_t { + Unknown = 0, + Interface = 1, + Function = 2, + Observer = 3, + }; + + Callback& operator=(const Callback& other) { + if (this != &other) { + clear(); + mType = other.mType; + switch (mType) { + case Type::Unknown: + break; + case Type::Interface: + mCallback.i = other.mCallback.i; + NS_ADDREF(mCallback.i); + break; + case Type::Function: + mCallback.c = other.mCallback.c; + break; + case Type::Observer: + mCallback.o = other.mCallback.o; + NS_ADDREF(mCallback.o); + break; + } + mName = other.mName; + mClosure = other.mClosure; + } + return *this; + } + + ~Callback() { clear(); } + + void clear() { + if (mType == Type::Interface) { + NS_RELEASE(mCallback.i); + } else if (mType == Type::Observer) { + NS_RELEASE(mCallback.o); + } + mType = Type::Unknown; + } + + void swap(Callback& other) { + std::swap(mType, other.mType); + std::swap(mCallback, other.mCallback); + std::swap(mName, other.mName); + std::swap(mClosure, other.mClosure); + } + + Type mType; + + union CallbackUnion { + nsTimerCallbackFunc c; + // These refcounted references are managed manually, as they are in a + // union + nsITimerCallback* MOZ_OWNING_REF i; + nsIObserver* MOZ_OWNING_REF o; + } mCallback; + + // |Name| is a tagged union type representing one of (a) nothing, (b) a + // string, or (c) a function. mozilla::Variant doesn't naturally handle the + // "nothing" case, so we define a dummy type and value (which is unused and + // so the exact value doesn't matter) for it. + typedef const int NameNothing; + typedef const char* NameString; + typedef nsTimerNameCallbackFunc NameFunc; + typedef mozilla::Variant Name; + static const NameNothing Nothing; + Name mName; - /// A raw function pointer and its closed-over state, along with its name for - /// logging purposes. - struct FuncCallback { - nsTimerCallbackFunc mFunc; void* mClosure; - const char* mName; }; - /// A callback defined by an owned closure and its name for logging purposes. - struct ClosureCallback { - std::function mFunc; - const char* mName; - }; - - using Callback = - mozilla::Variant; - nsresult InitCommon(uint32_t aDelayMS, uint32_t aType, Callback&& newCallback); @@ -133,9 +190,9 @@ class nsTimerImpl { void LogFiring(const Callback& aCallback, uint8_t aType, uint32_t aDelay); - nsresult InitWithClosureCallback(std::function&& aCallback, - const mozilla::TimeDuration& aDelay, - uint32_t aType, const char* aNameString); + nsresult InitWithFuncCallbackCommon(nsTimerCallbackFunc aFunc, void* aClosure, + uint32_t aDelay, uint32_t aType, + const Callback::Name& aName); // This weak reference must be cleared by the nsTimerImplHolder by calling // SetHolder(nullptr) before the holder is destroyed. @@ -182,16 +239,6 @@ class nsTimer final : public nsITimer { NS_DECL_THREADSAFE_ISUPPORTS NS_FORWARD_SAFE_NSITIMER(mImpl); - // NOTE: This constructor is not exposed on `nsITimer` as NS_FORWARD_SAFE_ - // does not support forwarding rvalue references. - nsresult InitWithClosureCallback(std::function&& aCallback, - const mozilla::TimeDuration& aDelay, - uint32_t aType, const char* aNameString) { - return mImpl ? mImpl->InitWithClosureCallback(std::move(aCallback), aDelay, - aType, aNameString) - : NS_ERROR_NULL_POINTER; - } - virtual size_t SizeOfIncludingThis( mozilla::MallocSizeOf aMallocSizeOf) const override;