Bug 141564 – [GTK] Memory leak from webkit_web_policy_decision_new()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Milan Crha]

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)

[Milan Crha]

Created attachment 246642 [details]
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.

[WebKit Commit Bot]

Attachment 246642 [details] 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.

[Carlos Garcia Campos]

Comment on attachment 246642 [details]
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.

[Carlos Garcia Campos]

Committed: <http://trac.webkit.org/changeset/182469>