126610 2014-01-07 17:54:50 -0800 Memory leak from windowScriptObjectAvailable 2014-01-30 19:58:51 -0800 1 1 1 Unclassified WebKit WebKit API 528+ (Nightly build) Unspecified Unspecified RESOLVED WORKSFORME P2 Normal --- 1 alex.christensen webkit-unassigned bfulgham ggaren mhahnenberg oldest_to_newest 965277 0 alex.christensen 2014-01-07 17:54:50 -0800 I'm working on finding a memory leak in Source/WebKit/win. We are getting a JSContextRef from the call to windowScriptObjectAvailable in WebFrameLoaderClient::dispatchDidClearWindowObjectInWorld. The JSContextRef has an extra reference to it that is not being dereferenced when we call WebView::close, and it's causing our JSObjectFinalizeCallback not to be called. I'm not sure where exactly to look, but it feels like a release of some sort should be added to WebView::close. 966094 1 alex.christensen 2014-01-09 15:20:54 -0800 Inspired by https://bugs.webkit.org/attachment.cgi?id=93818&action=prettypatch I added vm.heap.collectAllGarbage() to JSGlobalContextRelease and it fixed my problem without appearing to break anything. That's not optimal performance, and that's certainly not a good fix, but I think it's close. Is there a reason we aren't garbage collecting at all in JSGlobalContextRelease at all anymore? 966460 2 220862 alex.christensen 2014-01-10 10:55:54 -0800 Created attachment 220862 Patch 966464 3 alex.christensen 2014-01-10 10:58:55 -0800 This patch feels super hacky, and I'm sure there's a better way, and I'm sure there are reasons to not garbage collect when closing a WebView. I'm expecting an r-, but hopefully some insight about how to do this better. It would be really nice if there were a way to initiate garbage collection from a JSContextRef, so an API user can tell JavaScriptCore that now is a good time to garbage collect based on the state of my application that JavaScript doesn't know about. 966489 4 220862 ggaren 2014-01-10 11:57:35 -0800 Comment on attachment 220862 Patch A better way to do this is to call GCController::garbageCollectSoon(). Can you try that? GCController::garbageCollectSoon() does a better job of batching collection requests. 967931 5 alex.christensen 2014-01-14 11:06:07 -0800 Adding a call to GCController::garbageCollectNow to WebView::close solves my problem, but GCController::garbageCollectSoon does not. I'm not sure if every user of WebKit on Windows would want the lag of garbage collecting when closing a WebView. 967934 6 221181 alex.christensen 2014-01-14 11:15:23 -0800 Created attachment 221181 Patch 967979 7 alex.christensen 2014-01-14 12:41:06 -0800 I found a solution using the JavaScript API that works for me and doesn't force other WebKit users to garbage collect when they don't want to. I call JSReportExtraMemoryCost(ctx,1000000000); then JSGarbageCollect(ctx); and it garbage collects when I open the next WebKit widget. This is also hacky, but I can refine it on my end without changing WebKit. 967982 8 mhahnenberg 2014-01-14 12:51:34 -0800 Where is the extra reference to the JSContextRef coming from? Maybe the ref count is zero, but you're not sweeping anything so finalization never happens? Does the IncrementalSweeper work on Windows? collectAllGarbage also forces eager sweeping, which is why it would cause finalization to happen. 968011 9 alex.christensen 2014-01-14 13:31:42 -0800 (In reply to comment #7) > I call JSReportExtraMemoryCost(ctx,1000000000); then JSGarbageCollect(ctx); For the record, this is a terrible solution, and the garbage collector keeps the memory usage to a few megabytes when JSReportExtraMemoryCost is used correctly, which is reasonable. (In reply to comment #8) > Where is the extra reference to the JSContextRef coming from? Maybe the ref count is zero, but you're not sweeping anything so finalization never happens? Does the IncrementalSweeper work on Windows? I'm not sure, but the ref count of the JSContextRef is always between 2 and 4. Doing extra releases causes crashes, so there's probably something global that I wasn't expecting to be global. > > collectAllGarbage also forces eager sweeping, which is why it would cause finalization to happen. 968056 10 mhahnenberg 2014-01-14 14:48:29 -0800 I would do some further investigation into whether or not sweeping is being triggered in a timely manner. Try adding a printf or breakpoint inside MarkedBlock::sweep and see if things are being swept after the WebView is closed. Sweeping is when finalizers are run, so if a finalizer isn't running that should be the first thing to check. 974188 11 221181 alex.christensen 2014-01-30 19:58:51 -0800 Comment on attachment 221181 Patch I'm cleaning up old bugs. This does not need to go in. 220862 2014-01-10 10:55:54 -0800 2014-01-14 11:15:13 -0800 Patch bug-126610-20140110115552.patch text/plain 1164 alex.christensen SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2MTY1MikKKysrIFNvdXJjZS9XZWJLaXQv d2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE0LTAxLTEw ICBBbGV4IENocmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAg R2FyYmFnZSBjb2xsZWN0IHdoZW4gY2xvc2luZyBhIFdlYlZpZXcgdG8gcHJldmVudCBtZW1vcnkg bGVha3Mgd2hlbiB1c2luZyB0aGUgSmF2YVNjcmlwdCBBUEkuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjY2MTAKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFdlYlZpZXcuY3BwOgorICAgICAgICAoV2Vi Vmlldzo6Y2xvc2UpOgorICAgICAgICBHYXJiYWdlIGNvbGxlY3Qgd2hlbiBjbG9zaW5nIGEgV2Vi Vmlldy4KKwogMjAxNC0wMS0wOCAgcGVhdm9Ab3V0bG9vay5jb20gIDxwZWF2b0BvdXRsb29rLmNv bT4KIAogICAgICAgICBbV2luQ2Fpcm9dIENvbXBpbGUgZXJyb3IuCkluZGV4OiBTb3VyY2UvV2Vi S2l0L3dpbi9XZWJWaWV3LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L3dpbi9XZWJW aWV3LmNwcAkocmV2aXNpb24gMTYxMjY4KQorKysgU291cmNlL1dlYktpdC93aW4vV2ViVmlldy5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTcwMSw2ICs3MDEsOCBAQCBIUkVTVUxUIFNURE1FVEhPRENB TExUWVBFIFdlYlZpZXc6OmNsb3NlCiAKICAgICBtX2RpZENsb3NlID0gdHJ1ZTsKIAorICAgIGNv cmUodG9wTGV2ZWxGcmFtZSgpKS0+c2NyaXB0KCkuZ2xvYmFsT2JqZWN0KG1haW5UaHJlYWROb3Jt YWxXb3JsZCgpKS0+Z2xvYmFsRXhlYygpLT5oZWFwKCktPmNvbGxlY3RBbGxHYXJiYWdlKCk7CisK ICNpZiBVU0UoQUNDRUxFUkFURURfQ09NUE9TSVRJTkcpCiAgICAgc2V0QWNjZWxlcmF0ZWRDb21w b3NpdGluZyhmYWxzZSk7CiAjZW5kaWYK 221181 2014-01-14 11:15:23 -0800 2014-01-30 19:58:51 -0800 Patch bug-126610-20140114121726.patch text/plain 1298 alex.christensen SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2MTk4NSkKKysrIFNvdXJjZS9XZWJLaXQv d2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE0LTAxLTE0 ICBBbGV4IENocmlzdGVuc2VuICA8YWxleC5jaHJpc3RlbnNlbkBmbGV4c2ltLmNvbT4KKworICAg ICAgICBGaXhlZCBtZW1vcnkgbGVhayB3aGlsZSB1c2luZyBKYXZhU2NyaXB0IEFQSSBvbiBXaW5k b3dzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI2 NjEwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBX ZWJWaWV3LmNwcDoKKyAgICAgICAgKFdlYlZpZXc6OmNsb3NlKToKKyAgICAgICAgR2FyYmFnZSBj b2xsZWN0IHdoZW4gY2xvc2luZyBhIFdlYlZpZXcuCisKIDIwMTQtMDEtMTAgIEFuZGVycyBDYXJs c3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBUd2VhayBQcm9ncmVzc1RyYWNr ZXJDbGllbnQgZnVuY3Rpb25zCkluZGV4OiBTb3VyY2UvV2ViS2l0L3dpbi9XZWJWaWV3LmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L3dpbi9XZWJWaWV3LmNwcAkocmV2aXNpb24gMTYw Nzk1KQorKysgU291cmNlL1dlYktpdC93aW4vV2ViVmlldy5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTMxLDYgKzMxLDcgQEAKICNpbmNsdWRlICJDT01WYXJpYW50U2V0dGVyLmgiCiAjaW5jbHVkZSAi RE9NQ29yZUNsYXNzZXMuaCIKICNpbmNsdWRlICJGdWxsc2NyZWVuVmlkZW9Db250cm9sbGVyLmgi CisjaW5jbHVkZSAiR0NDb250cm9sbGVyLmgiCiAjaW5jbHVkZSAiTWFyc2hhbGxpbmdIZWxwZXJz LmgiCiAjaW5jbHVkZSAiU29mdExpbmtpbmcuaCIKICNpbmNsdWRlICJTdWJmcmFtZUxvYWRlci5o IgpAQCAtNzAxLDYgKzcwMiw4IEBAIEhSRVNVTFQgU1RETUVUSE9EQ0FMTFRZUEUgV2ViVmlldzo6 Y2xvc2UKIAogICAgIG1fZGlkQ2xvc2UgPSB0cnVlOwogCisgICAgZ2NDb250cm9sbGVyKCkuZ2Fy YmFnZUNvbGxlY3ROb3coKTsKKwogI2lmIFVTRShBQ0NFTEVSQVRFRF9DT01QT1NJVElORykKICAg ICBzZXRBY2NlbGVyYXRlZENvbXBvc2l0aW5nKGZhbHNlKTsKICNlbmRpZgo=