1555798 - BaseScalarInfo& in KeyedScalar could move, causing Problems

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Chris H-C :chutten]

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.

Comment 1

[Chris H-C :chutten]

Attachment: Bug 1555798 - Use a scalar name copy instead of a &BaseScalarInfo r?Dexter

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

Comment 2

[Chris H-C :chutten]

Attachment: Bug 1555798 - Test that registering and recording dynamic events and scalars together does not crash. r?Dexter

Depends on D33274

Comment 3

[Pulsebot]

Pushed by [e-mail]:
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

Comment 4

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/e4c9890a1f35
https://hg.mozilla.org/mozilla-central/rev/551589932a0a

Comment 5

[Chris H-C :chutten]

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:

Comment 6

[Julien Cristau [:jcristau]]

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

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c18af2278a4d
https://hg.mozilla.org/releases/mozilla-beta/rev/83245ccc42c0