Leaking about:telemetry window
Categories
(Toolkit :: Telemetry, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: mccr8, Assigned: chutten)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
I noticed this in my memory report:
974.77 MB (100.0%) -- explicit
├──446.11 MB (45.77%) ── heap-unclassified
├───97.33 MB (09.98%) -- window-objects
│ ├──71.40 MB (07.33%) -- top(none)
│ │ ├──55.03 MB (05.65%) -- ghost
│ │ │ ├──28.14 MB (02.89%) -- window(about:telemetry#search=separate)
│ │ │ │ ├──26.65 MB (02.73%) -- dom
│ │ │ │ │ ├──17.83 MB (01.83%) ── element-nodes
│ │ │ │ │ └───8.81 MB (00.90%) ++ (2 tiny)
│ │ │ │ └───1.49 MB (00.15%) ++ (3 tiny)
│ │ │ └──26.89 MB (02.76%) -- window(about:telemetry#search=memory_total)
│ │ │ ├──25.45 MB (02.61%) ++ dom
│ │ │ └───1.44 MB (00.15%) ++ (3 tiny)
It looks like the two times I opened about:telemetry and searched for something, it started leaking.
Assignee | ||
Comment 1•3 years ago
|
||
Well that's weird. How does one leak a whole window? I don't think we hold onto anything that would keep it alive.
Reporter | ||
Comment 2•3 years ago
|
||
I recorded a GC log, and this is the path holding the about:telemetry window alive:
0x2dcc6496858 [NonSyntacticVariablesObject <no private>]
--[observers]--> 0x2483a2c7ce48 [Array <no private>]
--[objectElements[16]]--> 0x3ac2425ab8c8 [Object <no private>]
--[callback]--> 0xc447d021a18 [Function render]
--[fun_environment]--> 0x18ecb2cf0b0 [LexicalEnvironment <no private>]
--[enclosing_environment]--> 0xf2ec1193b40 [Window <no private>]
From looking at the __URI__
field of the NSVO object, it looks like observers
is from Preferences.jsm.
Reporter | ||
Comment 3•3 years ago
|
||
Given that the name of the function registered as a callback is render
, I'm guessing this is due to this line of code: https://searchfox.org/mozilla-central/rev/1ba472c91bbde4b127f6e614ae72888ab59611cf/toolkit/content/aboutTelemetry.js#194
I guess detachObservers isn't being run.
Reporter | ||
Comment 4•3 years ago
|
||
Unsurprisingly, it looks like this specific render object is being registered for the two prefs in SETTINGS.
Reporter | ||
Comment 5•3 years ago
|
||
I can reproduce this in a clean profile in a debug build as follows:
- Open about:telemetry in a new tab.
- Close the tab.
- Open about:memory, run minimize memory usage, then get a memory report. about:telemetry is still in the memory report.
Reporter | ||
Comment 6•3 years ago
|
||
A browser-chrome test that opens and closes about:telemetry should be able to detect this leak.
Assignee | ||
Comment 7•3 years ago
|
||
Well heck. I can confirm (near as I can tell) that unload
and beforeunload
do not seem to run when closing about:
pages. (This may be causing issues for about:url-classifier
as well as I notice it, too, registers a handler.)
This is problematical as we need to have some guaranteed at-destroy method to unregister those preferences listeners. Preferences.jsm does not provide a weakreference-based API for listening, which would be ideal.
Looks like we have a couple of options here:
- Remove the prefs listener and only query them when rendering the overview. No more live-updates, but that's probably fine given the audience size and disposition of
about:telemetry
- Augment Preferences.jsm to accept weakreferences to observers. Could be a useful improvement.
Assignee | ||
Comment 8•3 years ago
|
||
unload listeners aren't reliable at the best of time, and for about: pages they
seem to never fire in my testing. There is no weakref-based listener setup for
prefs in JS and I don't want to build one, so this makes the prefs display in
about:telemetry only update on render, not live.
Comment 10•3 years ago
|
||
bugherder |
Description
•