Closed Bug 1555798 Opened 7 months ago Closed 7 months ago

BaseScalarInfo& in KeyedScalar could move, causing Problems

Categories

(Toolkit :: Telemetry, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- wontfix
firefox68 + fixed
firefox69 --- fixed

People

(Reporter: chutten, Assigned: chutten)

References

(Regression)

Details

Attachments

(2 files)

In bug 1451813 we started storing a reference to a BaseScalarInfo inside KeyedScalar. This was a mistake as, though compile-time "static" Scalars are immovable, runtime Dynamic Scalars can move their ScalarInfos whenever they'd like within gDynamicScalarInfo.

We should fix that and uplift as we can.

References to ScalarInfo objects are stable for all scalars registered
at compile-time, but not for those registered at runtime.

Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4c9890a1f35
Use a scalar name copy instead of a &BaseScalarInfo r=Dexter
https://hg.mozilla.org/integration/autoland/rev/551589932a0a
Test that registering and recording dynamic events and scalars together does not crash. r=Dexter
Assignee: nobody → chutten
Status: NEW → ASSIGNED
Blocks: 1556025
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Comment on attachment 9068963 [details]
Bug 1555798 - Test that registering and recording dynamic events and scalars together does not crash. r?Dexter

Beta/Release Uplift Approval Request

  • User impact if declined: Infrequent crash (bug 1555734) that impacts our confidence recording data in studies, experiments, privileged addons.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It looks good in Nightly and has a good test.
  • String changes made/needed:
Attachment #9068963 - Flags: approval-mozilla-beta?
Attachment #9068962 - Flags: approval-mozilla-beta?

Comment on attachment 9068962 [details]
Bug 1555798 - Use a scalar name copy instead of a &BaseScalarInfo r?Dexter

fix possible crash in telemetry code, approved for 68.0b7

Attachment #9068962 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9068963 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.