Memory leak due to circular reference Document->DOMTimer->ScheduledAction->[JS objects]->Document

• Component: DOM
• Status: RESOLVED FIXED

[Dmitry Titov]

DOMTimer::stop() will stop the timer but not release the ScheduledAction which holds to a JSFunction which can hold onto a bunch of JS wrappers They can keep a reference back to Document that owns the DOMTimer. Hence, refcount on a Document never goes to 0.
Fix is to delete the ScheduledAction in DOMTimer::stop().

[Dmitry Titov]

Created attachment 25813 [details]
proposed fix

[Dmitry Titov]

Created attachment 25814 [details]
repro file

repro file by [e-mail].
Set a breakpoint at DOMTimer::stop and ~DOMTimer. Load the file and then close the window.
You should see stop() called, then ~DOMTimer() called when Document is destroyed.
Before the fix applied, you see only stop(). After the patch with fix applied, both are invoked.

[Alexey Proskuryakov]

Comment on attachment 25813 [details]
proposed fix

r=me

I don't think the added null check in DOMTimer destructor is useful, but it isn't harmful either.

[Alexey Proskuryakov]

Committed revision 39066.

[Darin Adler]

Can we make a test case for this?

[Dmitry Titov]

(In reply to comment #5)
> Can we make a test case for this?

I think it's doable by adding a method to LayoutTestController (getJSObjectCount() which will do GCController::collect first). This way, it's possible to navigate say iframe to a test page and back and then see the number of objects alive.
But perhaps there is a simpler way that I don't see?

[Dmitry Titov]

Of course there is :-) Patch with test is here: bug 22730