Fronts, commands and a few class of inspector are leaked after toolbox is closed
Categories
(DevTools :: General, enhancement)
Tracking
(firefox94 fixed)
Tracking | Status | |
---|---|---|
firefox94 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
(Blocks 1 open bug)
Details
(Whiteboard: dt-perf-stability-mvp)
Attachments
(9 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
The memory test dedicated to toolbox open and close:
https://searchfox.org/mozilla-central/source/devtools/client/framework/test/allocations/browser_allocations_toolbox.js
is reporting many leaks today.
Today, on mozilla-central, it reports 44082 objects being leaked.
The leaks mostly come from bug 1539502, where we have a leak that is keeping the toolbox and/or React elements in memory. Both are being cross referenced, so it is unlear which one of the two are the real source of leak.
Until we figure out this leak, we can clear references here and there slightly better so that, if we happen to leak toolbox, inspector, targets, ... we aren't leaking much more.
We can especially focus on avoiding to leak: commands, fronts and sub classes of the inspector.
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Currently, we do leak the Toolbox/Inspector and some Documents when
closing the toolbox.
So that these classes are kept in memory, while not nullifying all its references
to client classes.
By adding these few nullications we can at least avoid leaking the fronts and commands.
(also fix a failure due to null markup.walker in browser_rules_add-rule-with-menu.js)
Assignee | ||
Comment 2•3 years ago
|
||
We still do leak Toolbox/Inspector and some Documents.
Unmounting the React components and nullifying references to them
seems to allow clearing a few React component instances.
We weren't unmounting the splitBox/toolsidebar, nor destroying ruleViewSideBar instance.
Assignee | ||
Comment 3•3 years ago
|
||
Assignee | ||
Comment 4•3 years ago
|
||
Assignee | ||
Comment 5•3 years ago
|
||
Also avoid creating a NodePicker during toolbox's destroy.
Assignee | ||
Comment 6•3 years ago
|
||
We do set resourceCommand
from onTargetAvailable, without ever clearing it.
If we happen to leak targets, we end up leaking commands.
Assignee | ||
Comment 7•3 years ago
|
||
Assignee | ||
Comment 8•3 years ago
|
||
Assignee | ||
Comment 9•3 years ago
•
|
||
Now another stack of patches to focus on toolbox leaks.
The number display "toolbox: xxxx" is the numbe of objects being reported as leaked by:
https://searchfox.org/mozilla-central/source/devtools/client/framework/test/allocations/browser_allocations_toolbox.js
Avoid leaking Commands/DevToolsClient/Fronts when closing the toolbox
- https://phabricator.services.mozilla.com/D125535
- toolbox: 43569
Ensure clearing references and unmounting all React components
- https://phabricator.services.mozilla.com/D125536
- toolbox: 33501
Ensure unregistering some DOM event listeners in the inspector
- https://phabricator.services.mozilla.com/D125537
- toolbox: 33384
Avoid leaking children fronts of target in case target is leaked
- https://phabricator.services.mozilla.com/D125538
- toolbox: 33249
Avoid leaking commands from NodePicker
- https://phabricator.services.mozilla.com/D125539
- toolbox: 33123
Avoid leaking commands from TargetFront
- https://phabricator.services.mozilla.com/D125540
- toolbox: 33123
Avoid leaking commands from toolbox
- https://phabricator.services.mozilla.com/D125541
- toolbox: 33123
Avoid leaking toolbox's redux store and ultimately targets
- https://phabricator.services.mozilla.com/D125542
- toolbox: 32328
Avoid leaking the rule panel via a click listener
- https://phabricator.services.mozilla.com/D125543
- toolbox: 31992
Assignee | ||
Comment 10•3 years ago
|
||
Assignee | ||
Comment 11•3 years ago
|
||
Comment 12•3 years ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/955138a73fa4 [devtools] Avoid leaking Commands/DevToolsClient/Fronts when closing the toolbox. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/9b1eb369d679 [devtools] Ensure clearing references and unmounting all React components. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/61dfd48f3409 [devtools] Ensure unregistering some DOM event listeners in the inspector. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/14619c743301 [devtools] Avoid leaking children fronts of target in case target is leaked. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/e9f40c89753f [devtools] Avoid leaking commands from NodePicker. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/22097d7a00c2 [devtools] Avoid leaking commands from TargetFront. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/331ffa0bd2f9 [devtools] Avoid leaking commands from toolbox. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/ac9a15005a5b [devtools] Avoid leaking toolbox's redux store and ultimately targets. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/509a0601481a [devtools] Avoid leaking the rule panel via a click listener. r=nchevobbe
Comment 13•3 years ago
|
||
Backed out for causing leaks.
Backout link: https://hg.mozilla.org/integration/autoland/rev/3c3b2dd39293e057034aee3272b58673111d27e4
Failure log:
-https://treeherder.mozilla.org/logviewer?job_id=351676284&repo=autoland&lineNumber=2785
-https://treeherder.mozilla.org/logviewer?job_id=351676278&repo=autoland&lineNumber=2545
-https://treeherder.mozilla.org/logviewer?job_id=351677655&repo=autoland&lineNumber=2196
-https://treeherder.mozilla.org/logviewer?job_id=351677623&repo=autoland&lineNumber=37127
Assignee | ||
Comment 14•3 years ago
|
||
It looks like it comes from bug 1728072, so let's try to land only these patches first.
With only patches from this bug, it looks green:
https://treeherder.mozilla.org/jobs?repo=try&revision=d8abd17ca97273bd690b00f93b64eb577a78e3ea
Comment 15•3 years ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/87456fe4679c [devtools] Avoid leaking Commands/DevToolsClient/Fronts when closing the toolbox. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/68037ba707f5 [devtools] Ensure clearing references and unmounting all React components. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/1ae31c877141 [devtools] Ensure unregistering some DOM event listeners in the inspector. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/bfe04d9f4203 [devtools] Avoid leaking children fronts of target in case target is leaked. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/d7c34f64e35e [devtools] Avoid leaking commands from NodePicker. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/e1f15379815f [devtools] Avoid leaking commands from TargetFront. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/55a0f637c042 [devtools] Avoid leaking commands from toolbox. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/3e8fac84e076 [devtools] Avoid leaking toolbox's redux store and ultimately targets. r=nchevobbe https://hg.mozilla.org/integration/autoland/rev/e7567d7b60a3 [devtools] Avoid leaking the rule panel via a click listener. r=nchevobbe
Comment 16•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/87456fe4679c
https://hg.mozilla.org/mozilla-central/rev/68037ba707f5
https://hg.mozilla.org/mozilla-central/rev/1ae31c877141
https://hg.mozilla.org/mozilla-central/rev/bfe04d9f4203
https://hg.mozilla.org/mozilla-central/rev/d7c34f64e35e
https://hg.mozilla.org/mozilla-central/rev/e1f15379815f
https://hg.mozilla.org/mozilla-central/rev/55a0f637c042
https://hg.mozilla.org/mozilla-central/rev/3e8fac84e076
https://hg.mozilla.org/mozilla-central/rev/e7567d7b60a3
Description
•