Leak nsGlobalWindow with localStorage, session history

RESOLVED FIXED in mozilla2.0b10

Status

()

Core
DOM
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: khuey)

Tracking

(Blocks: 2 bugs, {mlk, testcase})

Trunk
mozilla2.0b10
mlk, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [softblocker])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 500608 [details]
testcase
(Reporter)

Comment 1

7 years ago
Created attachment 500609 [details]
leak log
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
If this gets b+ I can take it.
I poked at this briefly, didn't quite figure the whole thing out, but made some progress:

There's what looks like a cycle between nsDOMStorageDBWrapper (which doesn't use the MOZ_COUNT_CTOR/MOZ_COUNT_DTOR macros!), an nsTimerImpl that it owns, an nsDOMStorage that is observing the timer (so the timer owns it), and a DOMStorageImpl.  Normally nsDOMStorageDBWrapper::StopTempTableFlushTimer destroys the timer but it never gets called.  The two places it can get called from are nsDOMStorageDBWrapper's dtor which isn't going to happen since we have a cycle and when the timer fires at [1].  We never end up in UnflushedDataExists, so I assume the timer never fires, but that's where I quit debugging.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/src/storage/nsDOMStorage.cpp#444
That's an nsDOMStorageManager that is observing the timer.
Or maybe it's not a cycle there, it doesn't look like nsDOMStorageManager owns anything.
nsGlobalWindow leaks are uncool, Honza, or Kyle, please do have a closer look.
blocking2.0: ? → final+
Whiteboard: [softblocker]
Created attachment 503742 [details] [diff] [review]
Testcase
Attachment #500608 - Attachment is obsolete: true
I've figured this one out.  Patch in the morning.
Assignee: nobody → khuey
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Created attachment 503806 [details] [diff] [review]
Pending storage events need to be cycle collected.
Comment on attachment 503806 [details] [diff] [review]
Pending storage events need to be cycle collected.

So, the issue here is that pending storage events form a cycle with the nsGlobalWindow.  This would not be an issue if we ever dispatched them, but in Jesse's testcase the window is never Unfrozen and the events are never fired.  Thus, we need to cycle collect the nsDOMStorageEventArray.
Attachment #503806 - Flags: review?(honzab.moz)
Attachment #503806 - Flags: review?(jst)
Attachment #503806 - Attachment is obsolete: true
Attachment #503806 - Flags: review?(jst)
Attachment #503806 - Flags: review?(honzab.moz)
Created attachment 503829 [details] [diff] [review]
Pending storage events need to be cycle collected.
Comment on attachment 503829 [details] [diff] [review]
Pending storage events need to be cycle collected.

This has a better test.  I managed to get it down to just one setTimeout.
Attachment #503829 - Flags: review?(honzab.moz)
Attachment #503829 - Flags: review?(jst)
Attachment #503742 - Attachment is obsolete: true

Updated

7 years ago
Attachment #503829 - Flags: review?(jst) → review+
Attachment #503829 - Flags: review?(honzab.moz)
http://hg.mozilla.org/mozilla-central/rev/57f3f4e9935f
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Depends on: 630588
Blocks: 735805
You need to log in before you can comment on or make changes to this bug.