Skip to content

P3A crash fix #29382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 5, 2025
Merged

P3A crash fix #29382

merged 1 commit into from
Jun 5, 2025

Conversation

DJAndries
Copy link
Collaborator

@DJAndries DJAndries commented Jun 4, 2025

Resolves brave/brave-browser#46549

  1. UpdateMetricValue gets called by the HandleHistogramChange loop in P3AService::Init, which creates the map elements:
    constellation_prep_log_stores_[*log_type]->UpdateValue(
  2. Start gets called, which calls RemoveObsoleteLogs. All elements in json_log_stores_ will be null if JSON reporting is deprecated (which is the case, via Griffin). It tries to call MetricLogStore::RemoveObsoleteLogs on null pointers

@DJAndries DJAndries requested a review from a team as a code owner June 4, 2025 04:17
@DJAndries DJAndries requested a review from SergeyZhukovsky June 4, 2025 04:17
@DJAndries DJAndries requested a review from a team as a code owner June 4, 2025 06:41
@DJAndries DJAndries force-pushed the p3a-crash-fix branch 2 times, most recently from 899ddbb to e672af0 Compare June 4, 2025 08:43
@SergeyZhukovsky
Copy link
Member

I DMed in slack, but the crash still happens with that change.

@DJAndries DJAndries force-pushed the p3a-crash-fix branch 2 times, most recently from 6923195 to 1ebc34a Compare June 4, 2025 17:49
@iefremov iefremov self-requested a review June 4, 2025 18:46
@iefremov
Copy link
Contributor

iefremov commented Jun 4, 2025

if there is no obvious fix, we should revert the whole thing while investigating

@DJAndries DJAndries force-pushed the p3a-crash-fix branch 4 times, most recently from 1e7aa6b to b89ba2f Compare June 4, 2025 20:38
@bsclifton
Copy link
Member

bsclifton commented Jun 4, 2025

PR we can revert if needed #28362

++ from me on reverting until there's a solid solution

@DJAndries DJAndries changed the title Attempted P3A crash fix P3A crash fix Jun 4, 2025
@SergeyZhukovsky
Copy link
Member

I verified the change in the current(updated) PR on Android and made sure there is no crash anymore.

Copy link
Member

@SergeyZhukovsky SergeyZhukovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ as I mentioned above, the changes are verified on Android and no issue there anymore.

@DJAndries DJAndries enabled auto-merge June 5, 2025 00:43
@DJAndries DJAndries merged commit d1464b6 into master Jun 5, 2025
18 checks passed
@DJAndries DJAndries deleted the p3a-crash-fix branch June 5, 2025 02:14
@github-actions github-actions bot added this to the 1.81.x - Nightly milestone Jun 5, 2025
@brave-builds
Copy link
Collaborator

Released in v1.81.50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash in P3AService
5 participants