SettingsLock objects leak by only calling destroyDOMRequestHandler during inner-window-closed

RESOLVED FIXED in Firefox 34, Firefox OS v2.1

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

25 Branch
2.1 S5 (26sep)
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, firefox33 unaffected, firefox34 fixed, firefox35 fixed, b2g-v1.4 unaffected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [systemsfe][MemShrink])

Attachments

(1 attachment)

[Blocking Requested - why for this release]:

SettingsLock objects never explicitly call destroyDOMRequestHelper when their existence ends (after receiving an Finalize::OK or Finalize::KO message). This means that the message manager will hold settings locks open until inner-window-destroyed. In the case of B2G, this means that locks existing in background apps (like system) will live forever, causing leaks.
blocking-b2g: 2.1? → 2.1+
Whiteboard: [systemsfe] → [systemsfe][MemShrink]
Created attachment 8493889 [details] [diff] [review]
Patch 1 (v1) - Call destroyDOMRequestHelper on SettingsLock after transaction ends
Assignee: nobody → kyle
Attachment #8493889 - Flags: review?(bent.mozilla)
Checked this by flashing flame, going through FTU, then running get_about_memory.py and looking for open settings locks (basically, grepping for "SettingsLock <no private>":

Before patch - 187 occurrences
After patch - 8 occurrences

I'm going to do root traces on the 8 occurrences after the patch to make sure those aren't leaks too, but there's a good chance those are just locks held by various objects as members or something.
Attachment #8493889 - Flags: review?(bent.mozilla) → review+
Comment on attachment 8493889 [details] [diff] [review]
Patch 1 (v1) - Call destroyDOMRequestHelper on SettingsLock after transaction ends

Approval Request Comment
[Feature/regressing bug #]: Bug 900551
[User impact if declined]: Memory leaks
[Describe test coverage new/current, TBPL]: It's believed that this currently takes down stability testing, so if that works...
[Risks and why]: None
[String/UUID change made/needed]: None
Attachment #8493889 - Flags: approval-mozilla-aurora?
Attachment #8493889 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Kyle landed this on Aurora.
https://hg.mozilla.org/releases/mozilla-aurora/rev/96e15bbed7ab
status-b2g-v2.1: affected → fixed
status-b2g-v2.2: affected → fixed
status-firefox33: --- → unaffected
status-firefox34: --- → fixed
status-firefox35: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/3c4b65dc595a
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.