22710 2008-12-06 04:54:36 -0800 Memory leak due to circular reference Document->DOMTimer->ScheduledAction->[JS objects]->Document 2008-12-07 18:05:59 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 dimich webkit-unassigned ap oldest_to_newest 101652 0 dimich 2008-12-06 04:54:36 -0800 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(). 101653 1 25813 dimich 2008-12-06 05:04:29 -0800 Created attachment 25813 proposed fix 101654 2 25814 dimich 2008-12-06 05:20:28 -0800 Created attachment 25814 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. 101657 3 25813 ap 2008-12-06 07:48:19 -0800 Comment on attachment 25813 proposed fix r=me I don't think the added null check in DOMTimer destructor is useful, but it isn't harmful either. 101658 4 ap 2008-12-06 08:04:41 -0800 Committed revision 39066. 101762 5 darin 2008-12-07 15:15:07 -0800 Can we make a test case for this? 101769 6 dimich 2008-12-07 15:52:38 -0800 (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? 101777 7 dimich 2008-12-07 18:05:59 -0800 Of course there is :-) Patch with test is here: bug 22730 25813 2008-12-06 05:04:29 -0800 2008-12-06 07:48:19 -0800 proposed fix patch.txt text/plain 1506 dimich SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzOTA2NSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMDgtMTItMDYgIERtaXRyeSBUaXRvdiAgPGRpbWljaEBjaHJvbWl1 bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg Rml4IG1lbW9yeSBsZWFrLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MjI3MTAKKworICAgICAgICAqIGJpbmRpbmdzL2pzL0RPTVRpbWVyLmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OkRPTVRpbWVyOjp+RE9NVGltZXIpOgorICAgICAgICAoV2ViQ29yZTo6 RE9NVGltZXI6OnN0b3ApOgorCiAyMDA4LTEyLTA2ICBEYXZpZCBLaWx6ZXIgIDxkZGtpbHplckBh cHBsZS5jb20+CiAKICAgICAgICAgQnVnIDIyNjY2OiBDbGVhbiB1cCBkYXRhIHN0cnVjdHVyZXMg dXNlZCB3aGVuIGNvbGxlY3RpbmcgVVJMcyBvZiBzdWJyZXNvdXJjZXMgZm9yIHdlYmFyY2hpdmVz CkluZGV4OiBXZWJDb3JlL2JpbmRpbmdzL2pzL0RPTVRpbWVyLmNwcAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBX ZWJDb3JlL2JpbmRpbmdzL2pzL0RPTVRpbWVyLmNwcAkocmV2aXNpb24gMzg5OTcpCisrKyBXZWJD b3JlL2JpbmRpbmdzL2pzL0RPTVRpbWVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTUsOCArNTUs MTAgQEAgRE9NVGltZXI6OkRPTVRpbWVyKFNjcmlwdEV4ZWN1dGlvbkNvbnRleAogCiBET01UaW1l cjo6fkRPTVRpbWVyKCkKIHsKLSAgICBKU0M6OkpTTG9jayBsb2NrKGZhbHNlKTsKLSAgICBkZWxl dGUgbV9hY3Rpb247CisgICAgaWYgKG1fYWN0aW9uKSB7CisgICAgICAgIEpTQzo6SlNMb2NrIGxv Y2soZmFsc2UpOworICAgICAgICBkZWxldGUgbV9hY3Rpb247CisgICAgfQogfQogCiB2b2lkIERP TVRpbWVyOjpmaXJlZCgpCkBAIC04Nyw2ICs4OSwxMiBAQCB2b2lkIERPTVRpbWVyOjpjb250ZXh0 RGVzdHJveWVkKCkKIHZvaWQgRE9NVGltZXI6OnN0b3AoKQogewogICAgIFRpbWVyQmFzZTo6c3Rv cCgpOworICAgIC8vIE5lZWQgdG8gcmVsZWFzZSBKUyBvYmplY3RzIHBvdGVudGlhbGx5IHByb3Rl Y3RlZCBieSBTY2hlZHVsZWRBY3Rpb24KKyAgICAvLyBiZWNhdXNlIHRoZXkgY2FuIGZvcm0gY2ly Y3VsYXIgcmVmZXJlbmNlcyBiYWNrIHRvIHRoZSBTY3JpcHRFeGVjdXRpb25Db250ZXh0CisgICAg Ly8gd2hpY2ggd2lsbCBjYXVzZSBhIG1lbW9yeSBsZWFrLgorICAgIEpTQzo6SlNMb2NrIGxvY2so ZmFsc2UpOworICAgIGRlbGV0ZSBtX2FjdGlvbjsKKyAgICBtX2FjdGlvbiA9IDA7CiB9CiAKIHZv aWQgRE9NVGltZXI6OnN1c3BlbmQoKSAK 25814 2008-12-06 05:20:28 -0800 2008-12-06 05:20:28 -0800 repro file timer-leak.html text/html 53 dimich PHNjcmlwdD4Kc2V0SW50ZXJ2YWwoZnVuY3Rpb24oKSB7fSwgMTAwMCk7Cjwvc2NyaXB0Pgo=