Massive memory leaks when capturing remote profiles via webide
Categories
(DevTools :: about:debugging, defect, P1)
Tracking
(firefox-esr60 unaffected, firefox66 wontfix, firefox67 verified, firefox68 verified)
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox66 | --- | wontfix |
firefox67 | --- | verified |
firefox68 | --- | verified |
People
(Reporter: jesup, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
(Keywords: memory-leak, regression, Whiteboard: [remote-debugging-reserve][qa-triaged])
Attachments
(2 files)
(deleted),
application/gzip
|
Details | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
When taking a number of profiles over several days with WebIDE against Fire devices, I noticed huge memory leaks. Main Process is using >7G of memory; 3.5G of it heap-unclassified, and 3.1G of it strings (often many thousands of copies of 10K+ long strings).
See the attached memory report.
I have several content processes examining the profiles; they all seem sane.
Base build is a recent inbound build.
Comment 1•6 years ago
|
||
The strings all start with:
Num RefCount Protocol Flags Type St Inode Path
Is it part of the profile being piped over the devtools protocol?
Comment 2•6 years ago
|
||
This kind of string seems to be about unix socket files, which we manipulate over here:
https://searchfox.org/mozilla-central/source/devtools/shared/adb/adb-device.js#31
Also, the about:memory report here is about the frontend, not the backend.
I'm not sure we do use unix socket for RDP transport from the frontend?
I believe that we do ask adb to open a TCP port and connect to it, right?
The leak seems huge, so unless we call this adb-device code a lot, that may be something else.
Julian, do you think someone from about:debugging team could look at this leak?
Assignee | ||
Comment 3•6 years ago
|
||
We are polling adb-devices in order to detect new compatible runtimes, so it's very likely linked to this. Will take a look. Thanks for the ni.
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
The client close() method is not used anywhere. Individual callers should be responsible for closing opened sockets if needed.
Removing this method and the _sockets array, we no longer leak strings when calling adb.updateRuntimes()
Assignee | ||
Updated•6 years ago
|
Comment 6•6 years ago
|
||
bugherder |
Comment 7•6 years ago
|
||
Is this something we should consider for backport to Beta or can it ride the trains?
Assignee | ||
Comment 8•6 years ago
|
||
Thanks for the suggestion, it should be easy to uplift and would be nice to have in 67.
Assignee | ||
Comment 9•6 years ago
|
||
Comment on attachment 9054166 [details]
Bug 1538708 - Stop storing sockets in adb-client.js;r=ochameau
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1492700
- User impact if declined: Constant memory leak after opening WebIDE (20kB/second). Leak is only cleaned after closing Firefox.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: - Open WebIDE
- Make sure the ADB extension is installed. WebIDE should download it and install it automatically. You can check this by going in "Project" > "Manage Extra Components"
- let WebIDE run for some time (anything longer than 3 seconds should be ok)
- close WebIDE
- open about:memory
- click on the GC & CC buttons a few times
- click on the Measure button in the top right
- filter by "inode"
ER: Should say "no results found"
AR: Some nodes are displayed for the "inode" filter
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Small JS patch, has been on nightly for 2 weeks. We are removing an unused API and an unused array.
- String changes made/needed:
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 10•6 years ago
|
||
Comment on attachment 9054166 [details]
Bug 1538708 - Stop storing sockets in adb-client.js;r=ochameau
Low risk, high reward, uplift approved for 67 beta 10. Let's get it verified by QA after it lands on beta, thanks.
Comment 11•6 years ago
|
||
bugherder uplift |
Comment 12•6 years ago
|
||
Verified as fixed on Firefox Nightly 68.0a1 (2019-04-12) and on Firefox 67.0b10 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.14 and on Ubuntu 18.04 x64.
Updated•6 years ago
|
Description
•