141564 2015-02-13 07:21:21 -0800 [GTK] Memory leak from webkit_web_policy_decision_new() 2015-04-07 06:10:57 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mcrha webkit-unassigned cgarcia commit-queue tpopela oldest_to_newest 1068912 0 mcrha 2015-02-13 07:21:21 -0800 I just noticed this memory leak warning when just running and closing wk-leaks.c from bug #118788 comment #5. ==8716== 8 bytes in 1 blocks are definitely lost in loss record 462 of 9,467 ==8716== at 0x4A070D7: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8716== by 0x5A49A4D: std::_Function_base::_Base_manager<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::{lambda(WebCore::PolicyAction)#1}>::_M_clone(std::_Any_data&, std::_Function_base::_Base_manager<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::{lambda(WebCore::PolicyAction)#1}> const&, std::integral_constant<bool, false>) (functional:1878) ==8716== by 0x5A49750: std::_Function_base::_Base_manager<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::{lambda(WebCore::PolicyAction)#1}>::_M_manager(std::_Any_data&, std::_Function_base::_Base_manager<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::{lambda(WebCore::PolicyAction)#1}> const&, std::_Manager_operation) (functional:1914) ==8716== by 0x5118AF1: std::function<void (WebCore::PolicyAction)>::function(std::function<void (WebCore::PolicyAction)> const&) (functional:2412) ==8716== by 0x513FB98: std::function<void (WebCore::PolicyAction)>::operator=(std::function<void (WebCore::PolicyAction)> const&) (functional:2243) ==8716== by 0x513F883: webkit_web_policy_decision_new (webkitwebpolicydecision.cpp:67) ==8716== by 0x5113D06: WebKit::FrameLoaderClient::dispatchDecidePolicyForNavigationAction(WebCore::NavigationAction const&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, std::function<void (WebCore::PolicyAction)>) (FrameLoaderClientGtk.cpp:446) ==8716== by 0x5A48B2B: WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>) (PolicyChecker.cpp:124) ==8716== by 0x5A12713: WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>) (FrameLoader.cpp:1489) ==8716== by 0x5A12052: WebCore::FrameLoader::load(WebCore::DocumentLoader*) (FrameLoader.cpp:1425) ==8716== by 0x5A11B69: WebCore::FrameLoader::load(WebCore::FrameLoadRequest const&) (FrameLoader.cpp:1375) ==8716== by 0x5139C26: webkit_web_frame_load_data(_WebKitWebFrame*, char const*, char const*, char const*, char const*, char const*) (webkitwebframe.cpp:699) ==8716== by 0x5139D76: webkit_web_frame_load_string (webkitwebframe.cpp:724) ==8716== by 0x51503B2: webkit_web_view_load_string (webkitwebview.cpp:4332) ==8716== by 0x40245F: load_page (wk-leaks.c:280) ==8716== by 0x4025C2: main (wk-leaks.c:313) 1069404 1 246642 mcrha 2015-02-16 04:07:32 -0800 Created attachment 246642 proposed patch The problem is that the priv->framePolicyFunction is not properly freed on GObject's finalize(), because it's a class inside a strucutre which is freed by GLib, not by C++. This is WebKit1 related, I didn't find webkit_web_policy_decision_new() in the git master, but as it's still relevant, then it'll be nice to have this fixed there. It would be also good to check WebKit2 code, whether it doesn't contain any similar issues. 1069405 2 commit-queue 2015-02-16 04:08:43 -0800 Attachment 246642 did not pass style-queue: Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style. 1069407 3 246642 cgarcia 2015-02-16 04:26:29 -0800 Comment on attachment 246642 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=246642&action=review Do not ask cq+ for wk1 patches since they will never apply in trunk. > webkitgtk-2.4.8/Source/WebKit/gtk/webkit/webkitwebpolicydecision.cpp:53 > + decision->priv->framePolicyFunction = 0; This should use nullptr instead of 0, I'll fix this before merging the patch in 2.4 branch. 1083299 4 cgarcia 2015-04-07 06:10:57 -0700 Committed: <http://trac.webkit.org/changeset/182469> 246642 2015-02-16 04:07:32 -0800 2015-02-16 04:26:29 -0800 proposed patch wk.patch text/plain 1742 mcrha ZGlmZiAtVTAgd2Via2l0Z3RrLTIuNC44L1NvdXJjZS9XZWJLaXQvZ3RrL0NoYW5nZUxvZy5tZW0t bGVhayB3ZWJraXRndGstMi40LjgvU291cmNlL1dlYktpdC9ndGsvQ2hhbmdlTG9nCi0tLSB3ZWJr aXRndGstMi40LjgvU291cmNlL1dlYktpdC9ndGsvQ2hhbmdlTG9nLm1lbS1sZWFrCTIwMTUtMDIt MTYgMTM6MDQ6MjguMDQyNDcyOTMzICswMTAwCisrKyB3ZWJraXRndGstMi40LjgvU291cmNlL1dl YktpdC9ndGsvQ2hhbmdlTG9nCTIwMTUtMDItMTYgMTM6MDY6MDEuMzEwNDY4OTgxICswMTAwCkBA IC0wLDAgKzEsOSBAQAorMjAxNS0wMi0xNiAgTWlsYW4gQ3JoYSAgPG1jcmhhQHJlZGhhdC5jb20+ CisKKyAgICAgICAgW0dUS10gTWVtb3J5IGxlYWsgZnJvbSB3ZWJraXRfd2ViX3BvbGljeV9kZWNp c2lvbl9uZXcoKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTQxNTY0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgKiB3ZWJraXQvd2Via2l0d2VicG9saWN5ZGVjaXNpb24uY3BwOgorCmRpZmYgLXVwIHdlYmtp dGd0ay0yLjQuOC9Tb3VyY2UvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0d2VicG9saWN5ZGVjaXNp b24uY3BwLm1lbS1sZWFrIHdlYmtpdGd0ay0yLjQuOC9Tb3VyY2UvV2ViS2l0L2d0ay93ZWJraXQv d2Via2l0d2VicG9saWN5ZGVjaXNpb24uY3BwCi0tLSB3ZWJraXRndGstMi40LjgvU291cmNlL1dl YktpdC9ndGsvd2Via2l0L3dlYmtpdHdlYnBvbGljeWRlY2lzaW9uLmNwcC5tZW0tbGVhawkyMDE1 LTAyLTE2IDEyOjU5OjA1LjkzNzQ4NjU4MSArMDEwMAorKysgd2Via2l0Z3RrLTIuNC44L1NvdXJj ZS9XZWJLaXQvZ3RrL3dlYmtpdC93ZWJraXR3ZWJwb2xpY3lkZWNpc2lvbi5jcHAJMjAxNS0wMi0x NiAxMjo0NzoyNi4zNTg1MTYyMjQgKzAxMDAKQEAgLTQ2LDkgKzQ2LDIzIEBAIHN0cnVjdCBfV2Vi S2l0V2ViUG9saWN5RGVjaXNpb25Qcml2YXRlIHsKICAgICBnYm9vbGVhbiBpc0NhbmNlbGxlZDsK IH07CiAKK3N0YXRpYyB2b2lkIHdlYmtpdF93ZWJfcG9saWN5X2RlY2lzaW9uX2ZpbmFsaXplKEdP YmplY3QgKm9iamVjdCkKK3sKKyAgICBXZWJLaXRXZWJQb2xpY3lEZWNpc2lvbiAqZGVjaXNpb24g PSBXRUJLSVRfV0VCX1BPTElDWV9ERUNJU0lPTiAob2JqZWN0KTsKKworICAgIGRlY2lzaW9uLT5w cml2LT5mcmFtZVBvbGljeUZ1bmN0aW9uID0gMDsKKworICAgIEdfT0JKRUNUX0NMQVNTICh3ZWJr aXRfd2ViX3BvbGljeV9kZWNpc2lvbl9wYXJlbnRfY2xhc3MpLT5maW5hbGl6ZSAob2JqZWN0KTsK K30KKwogc3RhdGljIHZvaWQgd2Via2l0X3dlYl9wb2xpY3lfZGVjaXNpb25fY2xhc3NfaW5pdChX ZWJLaXRXZWJQb2xpY3lEZWNpc2lvbkNsYXNzKiBkZWNpc2lvbkNsYXNzKQogeworICAgIEdPYmpl Y3RDbGFzcyAqb2JqZWN0X2NsYXNzOworCiAgICAgZ190eXBlX2NsYXNzX2FkZF9wcml2YXRlKGRl Y2lzaW9uQ2xhc3MsIHNpemVvZihXZWJLaXRXZWJQb2xpY3lEZWNpc2lvblByaXZhdGUpKTsKKwor ICAgIG9iamVjdF9jbGFzcyA9IEdfT0JKRUNUX0NMQVNTIChkZWNpc2lvbkNsYXNzKTsKKyAgICBv YmplY3RfY2xhc3MtPmZpbmFsaXplID0gd2Via2l0X3dlYl9wb2xpY3lfZGVjaXNpb25fZmluYWxp emU7CiB9CiAKIHN0YXRpYyB2b2lkIHdlYmtpdF93ZWJfcG9saWN5X2RlY2lzaW9uX2luaXQoV2Vi S2l0V2ViUG9saWN5RGVjaXNpb24qIGRlY2lzaW9uKQo=