622361 - Leak nsGlobalWindow with localStorage, session history

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Comment 1

[Jesse Ruderman]

Attachment: leak log

Comment 2

[Honza Bambas (:mayhemer)]

If this gets b+ I can take it.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

That's an nsDOMStorageManager that is observing the timer.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Or maybe it's not a cycle there, it doesn't look like nsDOMStorageManager owns anything.

Comment 6

[Johnny Stenback (:jst)]

nsGlobalWindow leaks are uncool, Honza, or Kyle, please do have a closer look.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Testcase

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I've figured this one out.  Patch in the morning.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Pending storage events need to be cycle collected.

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Pending storage events need to be cycle collected.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/57f3f4e9935f