Bug 141564

Summary:

[GTK] Memory leak from webkit_web_policy_decision_new()

Product:

WebKit

Reporter:

Milan Crha <mcrha>

Component:

WebKit Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cgarcia, commit-queue, tpopela

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch	cgarcia: review+, cgarcia: commit-queue-

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>