Closed
Bug 834470
Opened 11 years ago
Closed 11 years ago
Leaked AsyncPanZoomController and GestureEventListener due to non-CC'd cycle
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(blocking-b2g:-, firefox20 wontfix, firefox21 fixed, b2g18+ fixed, b2g18-v1.0.1 fixed)
RESOLVED
FIXED
blocking-b2g | - |
People
(Reporter: bjacob, Assigned: cjones)
References
Details
(Whiteboard: [found-by-refgraph][MemShrink])
Attachments
(1 file)
3.92 KB,
patch
|
roc
:
review+
jst
:
approval-mozilla-b2g18+
|
Details | Diff | Splinter Review |
This was found by refgraph, https://github.com/bjacob/mozilla-central/wiki There is a cycle of nsRefPtr's between AsyncPanZoomController and GestureEventListener that is not declared to the CC. The pointers are a nsRefPtr<GestureEventListener> and a nsRefPtr<AsyncPanZoomController>. Neither is traversed by the CC. This seems like a real leak: after a few minutes of usage of B2G/Otoro, I have two AsyncPanZoomController's and two GestureEventListener's, forming two such cycles; none of these cycles is referenced by any strong reference; and one of these two cycles is not even referenced anymore by any pointer at all from any other heap block, so it is almost certainly leaked.
Reporter | ||
Comment 1•11 years ago
|
||
Here's the refgraph summary for these cycles: The not-yet-leaked one: Cycle 33 has 2 vertices. Is NOT traversed by the CC! This cycle is NOT strong-referenced by outside! Types involved: mozilla::layers::AsyncPanZoomController / mozilla::layers::GestureEventListener type name: mozilla::layers::AsyncPanZoomController address: 0x40421820 size: 1072 bytes cycleIndex: 33 Cycle edges: nsRefPtr<mozilla::layers::GestureEventListener> to mozilla::layers::GestureEventListener at 0x440da860 NOT TRAVERSED BY CC backRefCount: 0 backWeakRefCount: 1 type name: mozilla::layers::GestureEventListener address: 0x440da860 size: 120 bytes cycleIndex: 33 Cycle edges: nsRefPtr<mozilla::layers::AsyncPanZoomController> to mozilla::layers::AsyncPanZoomController at 0x40421820 NOT TRAVERSED BY CC backRefCount: 0 backWeakRefCount: 0 The leaked one: Cycle 34 has 2 vertices. Is NOT traversed by the CC! This cycle is NOT referenced at all by outside heap blocks! Almost certainly leaked! Types involved: mozilla::layers::AsyncPanZoomController / mozilla::layers::GestureEventListener type name: mozilla::layers::AsyncPanZoomController address: 0x4807c020 size: 1072 bytes cycleIndex: 34 Cycle edges: nsRefPtr<mozilla::layers::GestureEventListener> to mozilla::layers::GestureEventListener at 0x4ab132c0 NOT TRAVERSED BY CC backRefCount: 0 backWeakRefCount: 0 type name: mozilla::layers::GestureEventListener address: 0x4ab132c0 size: 120 bytes cycleIndex: 34 Cycle edges: nsRefPtr<mozilla::layers::AsyncPanZoomController> to mozilla::layers::AsyncPanZoomController at 0x4807c020 NOT TRAVERSED BY CC backRefCount: 0 backWeakRefCount: 0
Updated•11 years ago
|
Whiteboard: [found-by-refgraph] → [found-by-refgraph][MemShrink]
Comment 2•11 years ago
|
||
Isn't AsyncPanZoomController used off-main-thread, so it can't be cycle collected.
Assignee | ||
Comment 3•11 years ago
|
||
Yes, these are non-main-thread objects. But let me confirm that we manually break this cycle.
Assignee | ||
Comment 4•11 years ago
|
||
It looks like we're not manually breaking that cycle currently :(. This could result in growing b2g memory usage, but it would require many many thousands of tabs opened to see MBs getting away from us.
Assignee: nobody → jones.chris.g
Blocks: slim-fast
Assignee | ||
Comment 5•11 years ago
|
||
There was another Axis <-> APZC cycle to break, but that one is trivial since APZC bounds the lifetime of Axis. (Yay conversions from Java ...)
Attachment #706187 -
Flags: review?(roc)
Assignee | ||
Comment 6•11 years ago
|
||
Use of the browser or any async-pan-zoomed apps can cause the b2g process to permanently "leak" memory, which eventually results in severe instability.
blocking-b2g: --- → tef?
Attachment #706187 -
Flags: review?(roc) → review+
Assignee | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7c845b323d0
Reporter | ||
Comment 8•11 years ago
|
||
While we're here: shouldn't refcounting on APZC be dropped? It doesn't seem like refcounting is actually how the APZC's lifetime is managed. Havingf unused refcounting seems dangerous, because if now someone arrived here, saw APZC being refcounted, and made code holding a nsRefPtr<APZC>, then the APZC would get destroyed as soon as that nsRefPtr<APZC> would go away (because nothing else addref'd it), which would be surprising, right?
Reporter | ||
Comment 9•11 years ago
|
||
Filed bug 834659 to have that separate conversation.
Comment 10•11 years ago
|
||
Not blocking, but tracking, and we'll approve the patch so it can land.
blocking-b2g: tef? → -
tracking-b2g18:
--- → +
Updated•11 years ago
|
Attachment #706187 -
Flags: approval-mozilla-b2g18+
Assignee | ||
Comment 11•11 years ago
|
||
Thanks for rebasing, jst! https://hg.mozilla.org/releases/mozilla-b2g18/rev/d5d6ef77f2d9
Status: NEW → RESOLVED
Closed: 11 years ago
status-b2g18:
--- → fixed
status-firefox20:
--- → wontfix
status-firefox21:
--- → fixed
Resolution: --- → FIXED
Comment 13•11 years ago
|
||
Batch edit: bugs fixed on b2g18 since 1/25 branch of v1.0 are fixed on v1.0.1
status-b2g18-v1.0.1:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•