Closed Bug 1538708 Opened 1 year ago Closed 1 year ago

Massive memory leaks when capturing remote profiles via webide

Categories

(DevTools :: about:debugging, defect, P1)

defect

Tracking

(firefox-esr60 unaffected, firefox66 wontfix, firefox67 verified, firefox68 verified)

VERIFIED FIXED
Firefox 68
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)

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.

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?

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?

Flags: needinfo?(jdescottes)

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.

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Component: WebIDE → about:debugging
Flags: needinfo?(jdescottes)
Priority: -- → P1
Whiteboard: [remote-debugging-reserve]

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()

See Also: → 1539834
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ed607a3b988e
Stop storing sockets in adb-client.js;r=ochameau
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

Is this something we should consider for backport to Beta or can it ride the trains?

Flags: needinfo?(jdescottes)

Thanks for the suggestion, it should be easy to uplift and would be nice to have in 67.

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:
Flags: needinfo?(jdescottes)
Attachment #9054166 - Flags: approval-mozilla-beta?
Flags: qe-verify?
Flags: qe-verify? → qe-verify+
Whiteboard: [remote-debugging-reserve] → [remote-debugging-reserve][qa-triaged]

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.

Attachment #9054166 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.