From d99ea245cecadba360d3aa0f94073c18eb3d1e46 Mon Sep 17 00:00:00 2001 From: Max Karolinskiy Date: Mon, 20 Apr 2026 10:32:55 -0400 Subject: [PATCH] [cr149] Fixes BraveWindowTracker dangling pointer. BraveWindowTracker is initialized in BraveBrowserMainExtraParts::PreMainMessageLoopRun. It uses base::ScopedObservation, which obseves browser collection which goes out of scope with GlobalFeatures. BraveWindowTracker needs to stop observing before then. Added HasInstance/ClearInstance to BraveWindowTracker modeling this behavior on upstream's ChromeBrowserMainExtraPartsMetrics and metrics::TabStatsTracker. [33240:46492:0417/173936.000:ERROR:base\allocator\partition_alloc_support.cc:683] A freed allocation is still referenced by a dangling pointer at exit, or at test end. Leaked raw_ptr/raw_ref could cause PartitionAlloc's quarantine memory bloat. Memory was released on: brave_browser_tests!base::allocator::`anonymous namespace'::DanglingRawPtrDetected [0x7ff7130f09ee+3ee] (base\allocator\partition_alloc_support.cc:438) brave_browser_tests!GlobalFeatures::~GlobalFeatures [0x7ff711aec13d+cd] (chrome\browser\global_features.cc:96) brave_browser_tests!BraveGlobalFeatures::~BraveGlobalFeatures [0x7ff710d19780+10] (brave\browser\brave_global_features.cc:12) brave_browser_tests!BrowserProcessImpl::~BrowserProcessImpl [0x7ff711b8ccb8+138] (chrome\browser\browser_process_impl.cc:565) brave_browser_tests!BraveBrowserProcessImpl::~BraveBrowserProcessImpl [0x7ff711b46128+3c8] (brave\browser\brave_browser_process_impl.cc:142) brave_browser_tests!BraveBrowserProcessImpl::~BraveBrowserProcessImpl [0x7ff711b47fa0+10] (brave\browser\brave_browser_process_impl.cc:142) brave_browser_tests!ChromeBrowserMainParts_ChromiumImpl::PostDestroyThreads [0x7ff711b52658+168] (chrome\browser\chrome_browser_main.cc:2291) brave_browser_tests!content::BrowserMainLoop::ShutdownThreadsAndCleanUp [0x7ff70d2638c7+7b7] (content\browser\browser_main_loop.cc:1282) brave_browser_tests!content::BrowserMainRunnerImpl::Shutdown [0x7ff70d265b86+106] (content\browser\browser_main_runner_impl.cc:177) ERROR:base\allocator\partition_alloc_support.cc:700] Dangling reference from: ERROR:base\allocator\partition_alloc_support.cc:701] brave_browser_tests!partition_alloc::internal::base::debug::CollectStackTrace [0x7ff713271b82+12] (base\allocator\partition_allocator\src\partition_alloc\partition_alloc_base\debug\stack_trace_win.cc:103) brave_browser_tests!base::internal::InstanceTracer::TraceImpl [0x7ff7132712ee+1fe] (base\allocator\partition_allocator\src\partition_alloc\pointers\instance_tracer.cc:66) brave_browser_tests!base::ScopedObservation::Observe [0x7ff707af12d8+128] (base\scoped_observation.h:117) brave_browser_tests!brave::BraveWindowTracker::BraveWindowTracker [0x7ff7119c682d+11d] (brave\browser\p3a\p3a_core_metrics.cc:60) brave_browser_tests!brave::BraveWindowTracker::CreateInstance [0x7ff7119c6de1+21] (brave\browser\p3a\p3a_core_metrics.cc:71) brave_browser_tests!BraveBrowserMainExtraParts::PreMainMessageLoopRun [0x7ff711a00915+185] (brave\browser\brave_browser_main_extra_parts.cc:104) brave_browser_tests!ChromeBrowserMainParts_ChromiumImpl::PreMainMessageLoopRun [0x7ff711b4fd60+60] (chrome\browser\chrome_browser_main.cc:1465) --- browser/brave_browser_main_extra_parts.cc | 16 ++++++++++++++-- browser/brave_browser_main_extra_parts.h | 1 + browser/p3a/p3a_core_metrics.cc | 17 +++++++++++++---- browser/p3a/p3a_core_metrics.h | 6 ++++-- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/browser/brave_browser_main_extra_parts.cc b/browser/brave_browser_main_extra_parts.cc index 5edea9036b6..c0c0135db76 100644 --- a/browser/brave_browser_main_extra_parts.cc +++ b/browser/brave_browser_main_extra_parts.cc @@ -51,8 +51,9 @@ void RecordInitialP3AValues() { g_browser_process->local_state()); // Record crash reporting status stats. - const bool crash_reports_enabled = g_browser_process->local_state()-> - GetBoolean(metrics::prefs::kMetricsReportingEnabled); + const bool crash_reports_enabled = + g_browser_process->local_state()->GetBoolean( + metrics::prefs::kMetricsReportingEnabled); UMA_HISTOGRAM_BOOLEAN("Brave.Core.CrashReportsEnabled", crash_reports_enabled); } @@ -105,3 +106,14 @@ void BraveBrowserMainExtraParts::PreMainMessageLoopRun() { #endif // !BUILDFLAG(IS_ANDROID) g_brave_browser_process->process_misc_metrics()->uptime_monitor()->Init(); } + +void BraveBrowserMainExtraParts::PostDestroyThreads() { +#if !BUILDFLAG(IS_ANDROID) + // Based on chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc + // The instance needs to delete itself as it uses BrowserCollection observer + // and the collection will go out of scope with the browser process. + if (brave::BraveWindowTracker::HasInstance()) { + brave::BraveWindowTracker::ClearInstance(); + } +#endif // !BUILDFLAG(IS_ANDROID) +} diff --git a/browser/brave_browser_main_extra_parts.h b/browser/brave_browser_main_extra_parts.h index cf3664e3706..b4555c64f11 100644 --- a/browser/brave_browser_main_extra_parts.h +++ b/browser/brave_browser_main_extra_parts.h @@ -22,6 +22,7 @@ class BraveBrowserMainExtraParts : public ChromeBrowserMainExtraParts { void PostBrowserStart() override; void PreMainMessageLoopRun() override; void PreProfileInit() override; + void PostDestroyThreads() override; }; #endif // BRAVE_BROWSER_BRAVE_BROWSER_MAIN_EXTRA_PARTS_H_ diff --git a/browser/p3a/p3a_core_metrics.cc b/browser/p3a/p3a_core_metrics.cc index 5fc6c458318..3cdf6761ac9 100644 --- a/browser/p3a/p3a_core_metrics.cc +++ b/browser/p3a/p3a_core_metrics.cc @@ -1,7 +1,7 @@ -/* Copyright 2019 The Brave Authors. All rights reserved. +/* Copyright (c) 2019 The Brave Authors. All rights reserved. * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ + * You can obtain one at https://mozilla.org/MPL/2.0/. */ #include "brave/browser/p3a/p3a_core_metrics.h" @@ -42,8 +42,7 @@ enum class WindowUsageStats { }; const char* GetPrefNameForProfile(Profile* profile) { - if (profile->IsIncognitoProfile() && - !profile->IsTor()) { + if (profile->IsIncognitoProfile() && !profile->IsTor()) { return kLastTimeIncognitoUsed; } return nullptr; @@ -71,6 +70,16 @@ void BraveWindowTracker::CreateInstance(PrefService* local_state) { g_brave_windows_tracker_instance = new BraveWindowTracker(local_state); } +bool BraveWindowTracker::HasInstance() { + return g_brave_windows_tracker_instance != nullptr; +} + +void BraveWindowTracker::ClearInstance() { + CHECK(g_brave_windows_tracker_instance); + delete g_brave_windows_tracker_instance; + g_brave_windows_tracker_instance = nullptr; +} + void BraveWindowTracker::RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterTimePref(kLastTimeIncognitoUsed, {}); registry->RegisterBooleanPref(kTorUsed, false); diff --git a/browser/p3a/p3a_core_metrics.h b/browser/p3a/p3a_core_metrics.h index 4be14bc2e7f..ae8973fd408 100644 --- a/browser/p3a/p3a_core_metrics.h +++ b/browser/p3a/p3a_core_metrics.h @@ -1,7 +1,7 @@ -/* Copyright 2019 The Brave Authors. All rights reserved. +/* Copyright (c) 2019 The Brave Authors. All rights reserved. * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ + * You can obtain one at https://mozilla.org/MPL/2.0/. */ #ifndef BRAVE_BROWSER_P3A_P3A_CORE_METRICS_H_ #define BRAVE_BROWSER_P3A_P3A_CORE_METRICS_H_ @@ -46,6 +46,8 @@ class BraveWindowTracker : public BrowserCollectionObserver { ~BraveWindowTracker() override; static void CreateInstance(PrefService* local_state); + static bool HasInstance(); + static void ClearInstance(); static void RegisterPrefs(PrefRegistrySimple* registry);