WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
22710
Memory leak due to circular reference Document->DOMTimer->ScheduledAction->[JS objects]->Document
https://bugs.webkit.org/show_bug.cgi?id=22710
Summary
Memory leak due to circular reference Document->DOMTimer->ScheduledAction->[J...
Dmitry Titov
Reported
2008-12-06 04:54:36 PST
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().
Attachments
proposed fix
(1.47 KB, patch)
2008-12-06 05:04 PST
,
Dmitry Titov
ap
: review+
Details
Formatted Diff
Diff
repro file
(53 bytes, text/html)
2008-12-06 05:20 PST
,
Dmitry Titov
no flags
Details
View All
Add attachment
proposed patch, testcase, etc.
Dmitry Titov
Comment 1
2008-12-06 05:04:29 PST
Created
attachment 25813
[details]
proposed fix
Dmitry Titov
Comment 2
2008-12-06 05:20:28 PST
Created
attachment 25814
[details]
repro file repro file by
ap@webkit.org
. 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 3
2008-12-06 07:48:19 PST
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
Comment 4
2008-12-06 08:04:41 PST
Committed revision 39066.
Darin Adler
Comment 5
2008-12-07 15:15:07 PST
Can we make a test case for this?
Dmitry Titov
Comment 6
2008-12-07 15:52:38 PST
(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
Comment 7
2008-12-07 18:05:59 PST
Of course there is :-) Patch with test is here:
bug 22730
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug