Bug 141564 - [GTK] Memory leak from webkit_web_policy_decision_new()
Summary: [GTK] Memory leak from webkit_web_policy_decision_new()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-13 07:21 PST by Milan Crha
Modified: 2015-04-07 06:10 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (1.70 KB, patch)
2015-02-16 04:07 PST, Milan Crha
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Milan Crha 2015-02-13 07:21:21 PST
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)
Comment 1 Milan Crha 2015-02-16 04:07:32 PST
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.
Comment 2 WebKit Commit Bot 2015-02-16 04:08:43 PST
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.
Comment 3 Carlos Garcia Campos 2015-02-16 04:26:29 PST
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.
Comment 4 Carlos Garcia Campos 2015-04-07 06:10:57 PDT
Committed: <http://trac.webkit.org/changeset/182469>