tor-browser

commit 0147760b374da72c6451351843ba53400d12a44a
parent 322bdb997f079b59bb5ec89b27af49db3b9404c1
Author: Paul Bone <paul@bone.id.au>
Date:   Tue,  9 Dec 2025 03:12:40 +0000

Bug 1996828 - Use reference counting for MemoryTelemetry r=toolkit-telemetry-reviewers,chutten

MemoryTelemetry was already ref counted, but it also had a static global
that was assumed to be never cleared/freed.  This patch makes it
entirely reference counted and places a reference in TelemetryImpl so
that TelemetryImpl can always access it.  It also requires threadsafe
reference counting since some of the memory telemetry is collected
off-thread.

Differential Revision: https://phabricator.services.mozilla.com/D274872

Diffstat:
M
js/xpconnect/src/XPCJSContext.cpp
 | 
5
++++-
M
js/xpconnect/tests/unit/test_js_memory_telemetry.js
 | 
6
++++++
M
toolkit/components/telemetry/core/Telemetry.cpp
 | 
24
++++++++++++++++++++----
M
xpcom/base/MemoryTelemetry.cpp
 | 
18
+++++++++++-------
M
xpcom/base/MemoryTelemetry.h
 | 
4
++--

5 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp
@@ -1536,7 +1536,10 @@ void XPCJSContext::AfterProcessTask(uint32_t aNewRecursionDepth) {
 
   // Poke the memory telemetry reporter
   if (AppShutdown::GetCurrentShutdownPhase() == ShutdownPhase::NotInShutdown) {
-    MemoryTelemetry::Get().Poke();
+    RefPtr<MemoryTelemetry> telemetry = MemoryTelemetry::Get();
+    if (telemetry) {
+      telemetry->Poke();
+    }
   }
 
   // This exception might have been set if we called an XPCWrappedJS that threw,
diff --git a/js/xpconnect/tests/unit/test_js_memory_telemetry.js b/js/xpconnect/tests/unit/test_js_memory_telemetry.js
@@ -8,6 +8,10 @@ add_task(function test_compartment_realm_counts() {
 
   Cu.forceShrinkingGC();
 
+  // MemoryTelemetry class needs to be created and initialised.  
+  Services.telemetry.earlyInit();
+  Services.telemetry.delayedInit();
+
   Services.telemetry.gatherMemory();
   let snapshot1 = Services.telemetry.getSnapshotForHistograms("main", true).parent;
 
@@ -50,4 +54,6 @@ add_task(function test_compartment_realm_counts() {
             "There must be more system realms than system compartments now");
 
   arr[0].x = 10; // Ensure the JS engine keeps |arr| alive until this point.
+  
+  Services.telemetry.shutdown();
 });
diff --git a/toolkit/components/telemetry/core/Telemetry.cpp b/toolkit/components/telemetry/core/Telemetry.cpp
@@ -168,6 +168,7 @@ class TelemetryImpl final : public nsITelemetry, public nsIMemoryReporter {
   Mutex mHashMutex MOZ_UNANNOTATED;
   Atomic<bool, SequentiallyConsistent> mCanRecordBase;
   Atomic<bool, SequentiallyConsistent> mCanRecordExtended;
+  RefPtr<MemoryTelemetry> mMemoryTelemetry;
 
   bool mCachedTelemetryData;
   uint32_t mLastShutdownTime;
@@ -1207,32 +1208,47 @@ TelemetryImpl::FlushBatchedChildTelemetry() {
 
 NS_IMETHODIMP
 TelemetryImpl::EarlyInit() {
-  (void)MemoryTelemetry::Get();
+  if (mMemoryTelemetry) {
+    // Don't do anything if EarlyInit ran already.
+    return NS_OK;
+  }
+  mMemoryTelemetry = MemoryTelemetry::Get();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TelemetryImpl::DelayedInit() {
-  MemoryTelemetry::Get().DelayedInit();
+  if (!mMemoryTelemetry) {
+    return NS_ERROR_FAILURE;
+  }
+  mMemoryTelemetry->DelayedInit();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TelemetryImpl::Shutdown() {
-  MemoryTelemetry::Get().Shutdown();
+  if (!mMemoryTelemetry) {
+    return NS_ERROR_FAILURE;
+  }
+  mMemoryTelemetry->Shutdown();
+  mMemoryTelemetry = nullptr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GatherMemory(JSContext* aCx, Promise** aResult) {
+  if (!mMemoryTelemetry) {
+    return NS_ERROR_FAILURE;
+  }
+
   ErrorResult rv;
   RefPtr<Promise> promise = Promise::Create(xpc::CurrentNativeGlobal(aCx), rv);
   if (rv.Failed()) {
     return rv.StealNSResult();
   }
 
-  MemoryTelemetry::Get().GatherReports(
+  mMemoryTelemetry->GatherReports(
       [promise]() { promise->MaybeResolve(JS::UndefinedHandleValue); });
 
   promise.forget(aResult);
diff --git a/xpcom/base/MemoryTelemetry.cpp b/xpcom/base/MemoryTelemetry.cpp
@@ -137,9 +137,9 @@ void MemoryTelemetry::Init() {
   }
 }
 
-/* static */ MemoryTelemetry& MemoryTelemetry::Get() {
-  static RefPtr<MemoryTelemetry> sInstance;
+static StaticRefPtr<MemoryTelemetry> sInstance;
 
+/* static */ RefPtr<MemoryTelemetry> MemoryTelemetry::Get() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!sInstance) {
@@ -147,7 +147,7 @@ void MemoryTelemetry::Init() {
     sInstance->Init();
     ClearOnShutdown(&sInstance);
   }
-  return *sInstance;
+  return sInstance;
 }
 
 void MemoryTelemetry::DelayedInit() {
@@ -235,6 +235,8 @@ nsresult MemoryTelemetry::Shutdown() {
 
   obs->RemoveObserver(this, kTopicShutdown);
 
+  sInstance = nullptr;
+
   return NS_OK;
 }
 
@@ -442,8 +444,10 @@ void MemoryTelemetry::GatherTotalMemory() {
         infos.AppendElement(info);
       });
 
+  RefPtr<MemoryTelemetry> self = this;
   mThreadPool->Dispatch(NS_NewRunnableFunction(
-      "MemoryTelemetry::GatherTotalMemory", [infos = std::move(infos)] {
+      "MemoryTelemetry::GatherTotalMemory",
+      [self = std::move(self), infos = std::move(infos)] {
         RefPtr<nsMemoryReporterManager> mgr =
             nsMemoryReporterManager::GetOrCreate();
         MOZ_RELEASE_ASSERT(mgr);
@@ -485,9 +489,9 @@ void MemoryTelemetry::GatherTotalMemory() {
 
         NS_DispatchToMainThread(NS_NewRunnableFunction(
             "MemoryTelemetry::FinishGatheringTotalMemory",
-            [mbTotal, childSizes = std::move(childSizes)] {
-              MemoryTelemetry::Get().FinishGatheringTotalMemory(mbTotal,
-                                                                childSizes);
+            [self = std::move(self), mbTotal,
+             childSizes = std::move(childSizes)] {
+              self->FinishGatheringTotalMemory(mbTotal, childSizes);
             }));
       }));
 }
diff --git a/xpcom/base/MemoryTelemetry.h b/xpcom/base/MemoryTelemetry.h
@@ -34,10 +34,10 @@ enum class ResponseRejectReason;
 class MemoryTelemetry final : public nsIObserver,
                               public nsSupportsWeakReference {
  public:
-  NS_DECL_ISUPPORTS
+  NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
-  static MemoryTelemetry& Get();
+  static RefPtr<MemoryTelemetry> Get();
 
   nsresult GatherReports(
       const std::function<void()>& aCompletionCallback = nullptr);