144721 2015-05-06 18:06:35 -0700 Worker threads leak WeakBlocks (as seen on leaks bot) 2015-05-07 11:33:55 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Performance P2 Normal --- 1 kling kling commit-queue ggaren oldest_to_newest 1092432 0 kling 2015-05-06 18:06:35 -0700 <rdar://problem/20848288> We've got about 750 KB worth of WeakBlocks leaked on the leaks bot right now. Boo. 1092434 1 252547 kling 2015-05-06 18:10:58 -0700 Created attachment 252547 Patch 1092609 2 252547 darin 2015-05-07 09:25:11 -0700 Comment on attachment 252547 Patch I think we return here and use std::unique_ptr for this. It would remove the need to explicitly call WeakBlock::destroy in Heap::sweepNextLogicallyEmptyWeakBlock. I know this is low-level code and std::unique_ptr might seem out of place, but I think it would cleanly prevent a mistake like this one! 1092638 3 kling 2015-05-07 10:42:41 -0700 (In reply to comment #2) > Comment on attachment 252547 [details] > Patch > > I think we return here and use std::unique_ptr for this. It would remove the > need to explicitly call WeakBlock::destroy in > Heap::sweepNextLogicallyEmptyWeakBlock. I know this is low-level code and > std::unique_ptr might seem out of place, but I think it would cleanly > prevent a mistake like this one! That's not a bad idea. I glossed over it because I assumed WeakBlocks were aligned to their block size, but it turns out that they're just plain fastMalloc()'ed objects. With a little care, we can fix these up to live in unique_ptr. Landing this right away to unscrew leaks bot though. 1092644 4 ggaren 2015-05-07 11:02:58 -0700 > I think we return here and use std::unique_ptr for this. It would remove the > need to explicitly call WeakBlock::destroy in > Heap::sweepNextLogicallyEmptyWeakBlock. I know this is low-level code and > std::unique_ptr might seem out of place, but I think it would cleanly > prevent a mistake like this one! I agree. Once upon a time it was not practical to unique_ptr these objects, but now it should be pretty trivial. 1092667 5 252547 commit-queue 2015-05-07 11:33:51 -0700 Comment on attachment 252547 Patch Clearing flags on attachment: 252547 Committed r183938: <http://trac.webkit.org/changeset/183938> 1092668 6 commit-queue 2015-05-07 11:33:55 -0700 All reviewed patches have been landed. Closing bug. 252547 2015-05-06 18:10:58 -0700 2015-05-07 11:33:51 -0700 Patch bug-144721.diff text/plain 1244 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDNlNTM5ZjguLjEwYWUyNmEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE1LTA1LTA2ICBBbmRyZWFzIEtsaW5n ICA8YWtsaW5nQGFwcGxlLmNvbT4KKworICAgICAgICBXb3JrZXIgdGhyZWFkcyBsZWFrIFdlYWtC bG9ja3MgKGFzIHNlZW4gb24gbGVha3MgYm90KQorICAgICAgICA8aHR0cHM6Ly93ZWJraXQub3Jn L2IvMTQ0NzIxPgorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMjA4NDgyODg+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTnVrZSBhbnkgcmVtYWluaW5n IGVtcHR5IFdlYWtCbG9ja3Mgd2hlbiB0aGUgSGVhcCBpcyBiZWluZyB0b3JuIGRvd24uCisgICAg ICAgIFRyeWluZyB0byBwZWVrIGludG8gdGhlc2UgYmxvY2tzIGFmdGVyIHRoZSBWTSBpcyBkZWFk IHdvdWxkIGJlIGEgYnVnIGFueXdheS4KKworICAgICAgICBUaGlzIGZpeGVzIGEgfjc1MCBLQiBs ZWFrIHNlZW4gb24gdGhlIGxlYWtzIGJvdC4KKworICAgICAgICAqIGhlYXAvSGVhcC5jcHA6Cisg ICAgICAgIChKU0M6OkhlYXA6On5IZWFwKToKKwogMjAxNS0wNC0wMiAgTXlsZXMgQy4gTWF4Zmll bGQgIDxtbWF4ZmllbGRAYXBwbGUuY29tPgogCiAgICAgICAgIEludHJvZHVjaW5nIHRoZSBQbGF0 Zm9ybSBBYnN0cmFjdGlvbiBMYXllciAoUEFMKQpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3Jp cHRDb3JlL2hlYXAvSGVhcC5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9IZWFwLmNw cAppbmRleCAxNDM2ZjAyLi43NzZjNjdhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvaGVhcC9IZWFwLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9IZWFwLmNw cApAQCAtMzY5LDYgKzM2OSw4IEBAIEhlYXA6OkhlYXAoVk0qIHZtLCBIZWFwVHlwZSBoZWFwVHlw ZSkKIAogSGVhcDo6fkhlYXAoKQogeworICAgIGZvciAoV2Vha0Jsb2NrKiBibG9jayA6IG1fbG9n aWNhbGx5RW1wdHlXZWFrQmxvY2tzKQorICAgICAgICBXZWFrQmxvY2s6OmRlc3Ryb3koYmxvY2sp OwogfQogCiBib29sIEhlYXA6OmlzUGFnZWRPdXQoZG91YmxlIGRlYWRsaW5lKQo=