Bug 1261702 - Make nsPerformanceStatsService::Dispose() idempotent,r=froydnj
Although I haven't been able to pinpoint why, it looks like
nsPerformanceStatsService::Dispose() may be called twice, which in
turn causes crashes. This patch makes sure that calling the method
twice is idempotent.
Also, just in case this was due to a typo in
AddObserver/RemoveObserver, this patch replaces the literal strings
used in both with constants.
MozReview-Commit-ID: 8fXO20r5xvO
--- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp
+++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp
@@ -131,16 +131,23 @@ GenerateUniqueGroupId(const JSRuntime* r
groupId.AssignLiteral("process: ");
groupId.AppendInt(processId);
groupId.AppendLiteral(", thread: ");
groupId.AppendInt(runtimeId);
groupId.AppendLiteral(", group: ");
groupId.AppendInt(uid);
}
+static const char* TOPICS[] = {
+ "profile-before-change",
+ "quit-application",
+ "quit-application-granted",
+ "xpcom-will-shutdown"
+};
+
} // namespace
/* ------------------------------------------------------
*
* class nsPerformanceObservationTarget
*
*/
@@ -633,16 +640,17 @@ PendingAlertsCollector::Dispose() {
* class nsPerformanceStatsService
*
*/
NS_IMPL_ISUPPORTS(nsPerformanceStatsService, nsIPerformanceStatsService, nsIObserver)
nsPerformanceStatsService::nsPerformanceStatsService()
: mIsAvailable(false)
+ , mDisposed(false)
#if defined(XP_WIN)
, mProcessId(GetCurrentProcessId())
#else
, mProcessId(getpid())
#endif
, mRuntime(xpc::GetJSRuntime())
, mUIdCounter(0)
, mTopGroup(nsPerformanceGroup::Make(mRuntime,
@@ -700,23 +708,28 @@ nsPerformanceStatsService::~nsPerformanc
void
nsPerformanceStatsService::Dispose()
{
// Make sure that we do not accidentally destroy `this` while we are
// cleaning up back references.
RefPtr<nsPerformanceStatsService> kungFuDeathGrip(this);
mIsAvailable = false;
+ if (mDisposed) {
+ // Make sure that we don't double-dispose.
+ return;
+ }
+ mDisposed = true;
+
// Disconnect from nsIObserverService.
nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
if (obs) {
- obs->RemoveObserver(this, "profile-before-change");
- obs->RemoveObserver(this, "quit-application");
- obs->RemoveObserver(this, "quit-application-granted");
- obs->RemoveObserver(this, "xpcom-will-shutdown");
+ for (size_t i = 0; i < mozilla::ArrayLength(TOPICS); ++i) {
+ mozilla::Unused << obs->RemoveObserver(this, TOPICS[i]);
+ }
}
// Clear up and disconnect from JSAPI.
js::DisposePerformanceMonitoring(mRuntime);
mozilla::Unused << js::SetStopwatchIsMonitoringCPOW(mRuntime, false);
mozilla::Unused << js::SetStopwatchIsMonitoringJank(mRuntime, false);
@@ -775,20 +788,19 @@ nsPerformanceStatsService::Init()
nsresult
nsPerformanceStatsService::InitInternal()
{
// Make sure that we release everything during shutdown.
// We are a bit defensive here, as we know that some strange behavior can break the
// regular shutdown order.
nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
if (obs) {
- obs->AddObserver(this, "profile-before-change", false);
- obs->AddObserver(this, "quit-application-granted", false);
- obs->AddObserver(this, "quit-application", false);
- obs->AddObserver(this, "xpcom-will-shutdown", false);
+ for (size_t i = 0; i < mozilla::ArrayLength(TOPICS); ++i) {
+ mozilla::Unused << obs->AddObserver(this, TOPICS[i], false);
+ }
}
// Connect to JSAPI.
if (!js::SetStopwatchStartCallback(mRuntime, StopwatchStartCallback, this)) {
return NS_ERROR_UNEXPECTED;
}
if (!js::SetStopwatchCommitCallback(mRuntime, StopwatchCommitCallback, this)) {
return NS_ERROR_UNEXPECTED;
--- a/toolkit/components/perfmonitoring/nsPerformanceStats.h
+++ b/toolkit/components/perfmonitoring/nsPerformanceStats.h
@@ -138,16 +138,21 @@ protected:
friend nsPerformanceGroup;
/**
* `false` until `Init()` and after `Dispose()`, `true` inbetween.
*/
bool mIsAvailable;
/**
+ * `true` once we have called `Dispose()`.
+ */
+ bool mDisposed;
+
+ /**
* A unique identifier for the process.
*
* Process HANDLE under Windows, pid under Unix.
*/
const uint64_t mProcessId;
/**
* The JS Runtime for the main thread.