65790 2011-08-05 14:18:48 -0700 [QT] Possible Leaks WKRetainPtr<> should Adopt allocated Copy 2011-08-07 12:42:40 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck webkit-unassigned joepeck psolanki webkit.review.bot oldest_to_newest 447415 0 joepeck 2011-08-05 14:18:48 -0700 shell> cd Source/ shell> a 'Retain.*?=.*?Copy' WebKit2/UIProcess/API/qt/qweberror.cpp 61: WKRetainPtr<WKStringRef> errorDomainPtr = adoptWK(WKErrorCopyDomain(d->error.get())); 80: WKRetainPtr<WKURLRef> failingURL = adoptWK(WKErrorCopyFailingURL(d->error.get())); WebKit2/UIProcess/qt/qwkhistory.cpp 76: WKRetainPtr<WKStringRef> title = WKBackForwardListItemCopyTitle(d->m_backForwardListItem.get()); 84: WKRetainPtr<WKURLRef> url = WKBackForwardListItemCopyURL(d->m_backForwardListItem.get()); These look like possible leaks. Since WKRetainPtr::operator= does a WKRetain and typically a WK*Copy*() will return a +1 allocation. I think the correct behavior here would be AdoptWK like: WebKit2/UIProcess/qt/qwkpreferences.cpp 93: WKRetainPtr<WKStringRef> stringRef(AdoptWK, WKPreferencesCopyStandardFontFamily(d->ref)); 447417 1 joepeck 2011-08-05 14:20:09 -0700 I should clarify. > WebKit2/UIProcess/API/qt/qweberror.cpp > 61: WKRetainPtr<WKStringRef> errorDomainPtr = adoptWK(WKErrorCopyDomain(d->error.get())); > 80: WKRetainPtr<WKURLRef> failingURL = adoptWK(WKErrorCopyFailingURL(d->error.get())); These look good because of the adoptWK(...). > WebKit2/UIProcess/qt/qwkhistory.cpp > 76: WKRetainPtr<WKStringRef> title = WKBackForwardListItemCopyTitle(d->m_backForwardListItem.get()); > 84: WKRetainPtr<WKURLRef> url = WKBackForwardListItemCopyURL(d->m_backForwardListItem.get()); These however don't look so good. 447421 2 103114 joepeck 2011-08-05 14:25:16 -0700 Created attachment 103114 [PATCH] Untested Proposed Fix I haven't tested this patch so I really suggest a WebKit2 / Qt developer take a look at this. I did not do a Qt build to test this, I just inspected the source. 447763 3 103114 webkit.review.bot 2011-08-07 12:42:36 -0700 Comment on attachment 103114 [PATCH] Untested Proposed Fix Clearing flags on attachment: 103114 Committed r92574: <http://trac.webkit.org/changeset/92574> 447764 4 webkit.review.bot 2011-08-07 12:42:40 -0700 All reviewed patches have been landed. Closing bug. 103114 2011-08-05 14:25:16 -0700 2011-08-07 12:42:36 -0700 [PATCH] Untested Proposed Fix proposed-fix.patch text/plain 1670 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAxYmJhM2Y4Li42OTdlMDA2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTEtMDgtMDUgIEpvc2VwaCBQZWNvcmFybyAgPGpvZXBlY2tAd2Via2l0Lm9yZz4KKwor ICAgICAgICBbUVRdIFBvc3NpYmxlIExlYWtzIFdLUmV0YWluUHRyPD4gc2hvdWxkIEFkb3B0IGFs bG9jYXRlZCBDb3B5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD02NTc5MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIEZpeCBhIGxlYWsgYnkgYWRvcHRpbmcgYW4gYWxsb2NhdGlvbiBpbnN0ZWFkIG9mIHJldGFp bmluZyBpdC4KKworICAgICAgICAqIFVJUHJvY2Vzcy9xdC9xd2toaXN0b3J5LmNwcDoKKyAgICAg ICAgKFFXS0hpc3RvcnlJdGVtOjp0aXRsZSk6CisgICAgICAgIChRV0tIaXN0b3J5SXRlbTo6dXJs KToKKwogMjAxMS0wOC0wMyAgTWFyayBSb3dlICA8bXJvd2VAYXBwbGUuY29tPgogCiAgICAgICAg IEJyaW5nIHNvbWUgb3JkZXIgdG8gRmVhdHVyZURlZmluZXMueGNjb25maWcgdG8gbWFrZSBpdCBl YXNpZXIgdG8gZm9sbG93LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0 L3F3a2hpc3RvcnkuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0L3F3a2hpc3Rvcnku Y3BwCmluZGV4IGIxM2ExMDAuLjZjMTQyNWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJ UHJvY2Vzcy9xdC9xd2toaXN0b3J5LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv cXQvcXdraGlzdG9yeS5jcHAKQEAgLTczLDcgKzczLDcgQEAgUVN0cmluZyBRV0tIaXN0b3J5SXRl bTo6dGl0bGUoKSBjb25zdAogewogICAgIGlmICghZC0+bV9iYWNrRm9yd2FyZExpc3RJdGVtKQog ICAgICAgICByZXR1cm4gUVN0cmluZygpOwotICAgIFdLUmV0YWluUHRyPFdLU3RyaW5nUmVmPiB0 aXRsZSA9IFdLQmFja0ZvcndhcmRMaXN0SXRlbUNvcHlUaXRsZShkLT5tX2JhY2tGb3J3YXJkTGlz dEl0ZW0uZ2V0KCkpOworICAgIFdLUmV0YWluUHRyPFdLU3RyaW5nUmVmPiB0aXRsZShBZG9wdFdL LCBXS0JhY2tGb3J3YXJkTGlzdEl0ZW1Db3B5VGl0bGUoZC0+bV9iYWNrRm9yd2FyZExpc3RJdGVt LmdldCgpKSk7CiAgICAgcmV0dXJuIFdLU3RyaW5nQ29weVFTdHJpbmcodGl0bGUuZ2V0KCkpOwog fQogCkBAIC04MSw3ICs4MSw3IEBAIFFVcmwgUVdLSGlzdG9yeUl0ZW06OnVybCgpIGNvbnN0CiB7 CiAgICAgaWYgKCFkLT5tX2JhY2tGb3J3YXJkTGlzdEl0ZW0pCiAgICAgICAgIHJldHVybiBRVXJs KCk7Ci0gICAgV0tSZXRhaW5QdHI8V0tVUkxSZWY+IHVybCA9IFdLQmFja0ZvcndhcmRMaXN0SXRl bUNvcHlVUkwoZC0+bV9iYWNrRm9yd2FyZExpc3RJdGVtLmdldCgpKTsKKyAgICBXS1JldGFpblB0 cjxXS1VSTFJlZj4gdXJsKEFkb3B0V0ssIFdLQmFja0ZvcndhcmRMaXN0SXRlbUNvcHlVUkwoZC0+ bV9iYWNrRm9yd2FyZExpc3RJdGVtLmdldCgpKSk7CiAgICAgcmV0dXJuIFdLVVJMQ29weVFVcmwo dXJsLmdldCgpKTsKIH0KIAo=