Bug 1545120 - Simplify this callback management stuff a little. r=froydnj, a=lizzard
Differential Revision:
https://phabricator.services.mozilla.com/D28390
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -257,17 +257,18 @@ nsTimer::Release(void) {
return count;
}
nsTimerImpl::nsTimerImpl(nsITimer* aTimer)
: mHolder(nullptr),
mGeneration(0),
mITimer(aTimer),
- mMutex("nsTimerImpl::mMutex") {
+ mMutex("nsTimerImpl::mMutex"),
+ mFiring(0) {
// XXXbsmedberg: shouldn't this be in Init()?
mEventTarget = mozilla::GetCurrentThreadEventTarget();
}
// static
nsresult nsTimerImpl::Startup() {
nsresult rv;
@@ -418,18 +419,17 @@ void nsTimerImpl::CancelImpl(bool aClear
gThread->RemoveTimer(this);
}
cbTrash.swap(mCallback);
++mGeneration;
// Don't clear this if we're firing; once Fire returns, we'll get this call
// again.
- if (aClearITimer &&
- (mCallbackDuringFire.mType == Callback::Type::Unknown)) {
+ if (aClearITimer && !mFiring) {
MOZ_RELEASE_ASSERT(
mITimer,
"mITimer was nulled already! "
"This indicates that someone has messed up the refcount on nsTimer!");
timerTrash.swap(mITimer);
}
}
}
@@ -522,27 +522,29 @@ nsresult nsTimerImpl::GetAllowedEarlyFir
*aValueOut = gThread ? gThread->AllowedEarlyFiringMicroseconds() : 0;
return NS_OK;
}
void nsTimerImpl::Fire(int32_t aGeneration) {
uint8_t oldType;
uint32_t oldDelay;
TimeStamp oldTimeout;
+ Callback callbackDuringFire;
nsCOMPtr<nsITimer> kungFuDeathGrip;
{
// 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;
}
- mCallbackDuringFire.swap(mCallback);
+ ++mFiring;
+ callbackDuringFire = mCallback;
oldType = mType;
oldDelay = mDelay.ToMilliseconds();
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.
kungFuDeathGrip = mITimer;
}
@@ -563,49 +565,52 @@ void nsTimerImpl::Fire(int32_t aGenerati
("[this=%p] actual delay time %4dms\n", this, oldDelay + d));
MOZ_LOG(GetTimerLog(), LogLevel::Debug,
("[this=%p] (mType is %d) -------\n", this, oldType));
MOZ_LOG(GetTimerLog(), LogLevel::Debug,
("[this=%p] delta %4dms\n", this, d));
}
if (MOZ_LOG_TEST(GetTimerFiringsLog(), LogLevel::Debug)) {
- LogFiring(mCallbackDuringFire, oldType, oldDelay);
+ LogFiring(callbackDuringFire, oldType, oldDelay);
}
- switch (mCallbackDuringFire.mType) {
+ switch (callbackDuringFire.mType) {
case Callback::Type::Function:
- mCallbackDuringFire.mCallback.c(mITimer, mCallbackDuringFire.mClosure);
+ callbackDuringFire.mCallback.c(mITimer, callbackDuringFire.mClosure);
break;
case Callback::Type::Interface:
- mCallbackDuringFire.mCallback.i->Notify(mITimer);
+ callbackDuringFire.mCallback.i->Notify(mITimer);
break;
case Callback::Type::Observer:
- mCallbackDuringFire.mCallback.o->Observe(mITimer, NS_TIMER_CALLBACK_TOPIC,
- nullptr);
+ callbackDuringFire.mCallback.o->Observe(mITimer, NS_TIMER_CALLBACK_TOPIC,
+ nullptr);
break;
default:;
}
- Callback trash; // Swap into here to dispose of callback after the unlock
MutexAutoLock lock(mMutex);
- if (aGeneration == mGeneration && IsRepeating()) {
- // Repeating timer has not been re-init or canceled; reschedule
- mCallbackDuringFire.swap(mCallback);
- if (IsSlack()) {
- mTimeout = TimeStamp::Now() + mDelay;
+ if (aGeneration == mGeneration) {
+ if (IsRepeating()) {
+ // Repeating timer has not been re-init or canceled; reschedule
+ if (IsSlack()) {
+ mTimeout = TimeStamp::Now() + mDelay;
+ } else {
+ mTimeout = mTimeout + mDelay;
+ }
+ if (gThread) {
+ gThread->AddTimer(this);
+ }
} else {
- mTimeout = mTimeout + mDelay;
- }
- if (gThread) {
- gThread->AddTimer(this);
+ // Non-repeating timer that has not been re-scheduled. Clear.
+ mCallback.clear();
}
}
- mCallbackDuringFire.swap(trash);
+ --mFiring;
MOZ_LOG(GetTimerLog(), LogLevel::Debug,
("[this=%p] Took %fms to fire timer callback\n", this,
(TimeStamp::Now() - now).ToMilliseconds()));
}
#if defined(HAVE_DLADDR) && defined(HAVE___CXA_DEMANGLE)
#define USE_DLADDR 1
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -62,40 +62,68 @@ class nsTimerImpl {
int32_t GetGeneration() { return mGeneration; }
struct Callback {
Callback() : mType(Type::Unknown), mName(Nothing), mClosure(nullptr) {
mCallback.c = nullptr;
}
- Callback(const Callback& other) = delete;
- Callback& operator=(const Callback& other) = delete;
+ Callback(const Callback& other) : Callback() { *this = other; }
+
+ enum class Type : uint8_t {
+ Unknown = 0,
+ Interface = 1,
+ Function = 2,
+ Observer = 3,
+ };
- ~Callback() {
+ 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);
}
- enum class Type : uint8_t {
- Unknown = 0,
- Interface = 1,
- Function = 2,
- Observer = 3,
- };
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;
@@ -118,20 +146,16 @@ class nsTimerImpl {
nsresult InitCommon(uint32_t aDelayMS, uint32_t aType,
Callback&& newCallback);
nsresult InitCommon(const mozilla::TimeDuration& aDelay, uint32_t aType,
Callback&& newCallback);
Callback& GetCallback() {
mMutex.AssertCurrentThreadOwns();
- if (mCallback.mType == Callback::Type::Unknown) {
- return mCallbackDuringFire;
- }
-
return mCallback;
}
bool IsRepeating() const {
static_assert(nsITimer::TYPE_ONE_SHOT < nsITimer::TYPE_REPEATING_SLACK,
"invalid ordering of timer types!");
static_assert(
nsITimer::TYPE_REPEATING_SLACK < nsITimer::TYPE_REPEATING_PRECISE,
@@ -188,17 +212,18 @@ class nsTimerImpl {
#endif
static double sDeltaSum;
static double sDeltaSumSquared;
static double sDeltaNum;
RefPtr<nsITimer> mITimer;
mozilla::Mutex mMutex;
Callback mCallback;
- Callback mCallbackDuringFire;
+ // Counter because in rare cases we can Fire reentrantly
+ unsigned int mFiring;
};
class nsTimer final : public nsITimer {
virtual ~nsTimer();
public:
nsTimer() : mImpl(new nsTimerImpl(this)) {}