86609 2012-05-16 04:59:03 -0700 [EFL] Memory leak in RenderThemeEfl 2012-05-16 09:00:14 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 naginenis naginenis antognolli+webkit gustavo gyuyoung.kim gyuyoung.kim lucas.de.marchi mrobinson pnormand rakuco webkit.review.bot oldest_to_newest 625283 0 naginenis 2012-05-16 04:59:03 -0700 Valgrind reports a memory leak in RenderThemeEfl. I guess the cairo surface needs to be freed when the RenderTheme is destroyed. ==11717== ==11717== 368 bytes in 1 blocks are definitely lost in loss record 9,399 of 11,096 ==11717== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==11717== by 0x5073D6F: _cairo_image_surface_create_for_pixman_image (cairo-image-surface.c:158) ==11717== by 0x5076698: _cairo_image_surface_create_with_pixman_format (cairo-image-surface.c:335) ==11717== by 0x9D3A0E5: WebCore::RenderThemeEfl::themePartCacheEntrySurfaceCreate(WebCore::RenderThemeEfl::ThemePartCacheEntry*) (RenderThemeEfl.cpp:135) ==11717== by 0x9D3A430: WebCore::RenderThemeEfl::cacheThemePartNew(WebCore::FormType, WebCore::IntSize const&) (RenderThemeEfl.cpp:190) ==11717== by 0x9D3A766: WebCore::RenderThemeEfl::cacheThemePartGet(WebCore::FormType, WebCore::IntSize const&) (RenderThemeEfl.cpp:259) ==11717== by 0x9D3AA8E: WebCore::RenderThemeEfl::paintThemePart(WebCore::RenderObject*, WebCore::FormType, WebCore::PaintInfo const&, WebCore::IntRect const&) (RenderThemeEfl.cpp:319) ==11717== by 0x9D3D448: WebCore::RenderThemeEfl::paintMenuList(WebCore::RenderObject*, WebCore::PaintInfo const&, WebCore::IntRect const&) (RenderThemeEfl.cpp:869) ==11717== by 0x9612898: WebCore::RenderTheme::paint(WebCore::RenderObject*, WebCore::PaintInfo const&, WebCore::IntRect const&) (RenderTheme.cpp:292) ==11717== by 0x950FEAF: WebCore::RenderBox::paintBoxDecorations(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBox.cpp:960) ==11717== by 0x94B52E1: WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2758) ==11717== by 0x94B3217: WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2528) ==11717== by 0x948A552: WebCore::InlineBox::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&, WebCore::FractionalLayoutUnit, WebCore::FractionalLayoutUnit) (InlineBox.cpp:232) ==11717== by 0x9490622: WebCore::InlineFlowBox::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&, WebCore::FractionalLayoutUnit, WebCore::FractionalLayoutUnit) (InlineFlowBox.cpp:1078) ==11717== by 0x962E48E: WebCore::RootInlineBox::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&, WebCore::FractionalLayoutUnit, WebCore::FractionalLayoutUnit) (RootInlineBox.cpp:206) ==11717== by 0x959F959: WebCore::RenderLineBoxList::paint(WebCore::RenderBoxModelObject*, WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) const (RenderLineBoxList.cpp:262) ==11717== by 0x94B49C0: WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2672) ==11717== by 0x94B5466: WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2782) ==11717== by 0x94B3217: WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2528) ==11717== by 0x94B4E45: WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2716) ==11717== by 0x94B49D9: WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2674) ==11717== by 0x94B5466: WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2782) ==11717== by 0x94B3217: WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2528) ==11717== by 0x94B4E45: WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2716) ==11717== by 0x94B49D9: WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2674) ==11717== by 0x94B5466: WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2782) ==11717== by 0x94B3217: WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&) (RenderBlock.cpp:2528) ==11717== by 0x958EA3D: WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::Re ==11717== by 0x958E34A: WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject* ==11717== by 0x958E24A: WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegi ==11717== by 0x958F069: WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, We ==11717== by 0x958ECAD: WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::Re ==11717== by 0x958E34A: WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject* ==11717== by 0x958E24A: WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegi ==11717== by 0x958F069: WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, We ==11717== by 0x958ECAD: WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::Re ==11717== by 0x958E34A: WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject* ==11717== by 0x958E24A: WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegi ==11717== by 0x958D6E4: WebCore::RenderLayer::paint(WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, unsigned int) (RenderLa ==11717== by 0x932E134: WebCore::FrameView::paintContents(WebCore::GraphicsContext*, WebCore::IntRect const&) (FrameView.cpp:3093) ==11717== by 0x4FDA34B: ewk_view_paint_context_paint_contents (ewk_view.cpp:2765) ==11717== by 0x4FE7967: _ewk_view_single_smart_repaints_process(_Ewk_View_Smart_Data*) (ewk_view_single.cpp:314) ==11717== by 0x4FC2F81: _ewk_view_smart_calculate(_Evas_Object*) (ewk_view.cpp:982) ==11717== by 0x4E79D9B: evas_call_smarts_calculate (evas_object_smart.c:688) ==11717== by 0x4EA0EC1: evas_render_updates_internal (evas_render.c:1359) ==11717== by 0x41850D4: _ecore_evas_x_render (ecore_evas_x.c:281) ==11717== by 0x41823C0: _ecore_evas_idle_enter (ecore_evas.c:59) ==11717== by 0x41459B5: _ecore_idle_enterer_call (ecore_private.h:263) ==11717== by 0x41470CA: _ecore_main_loop_iterate_internal (ecore_main.c:1789) ==11717== by 0x4147726: ecore_main_loop_begin (ecore_main.c:906) 625300 1 142226 naginenis 2012-05-16 05:29:03 -0700 Created attachment 142226 Fix memory leak Free the surface with cairo_surface_destroy. 625323 2 142226 gyuyoung.kim 2012-05-16 05:45:54 -0700 Comment on attachment 142226 Fix memory leak Although I'm not expert this area, according to cairo API document, cairo_surface_destroy() decreases reference count(). Then, if reference count is zero, it frees resource by calling cairo_surface_finish(). In other hands, cairo_surface_finish() drops resource to external resource directly. Could you let me know why cairo_surface_destroy() can fix memory leak ? 625351 3 naginenis 2012-05-16 06:38:33 -0700 (In reply to comment #2) > (From update of attachment 142226 [details]) > Although I'm not expert this area, according to cairo API document, cairo_surface_destroy() decreases reference count(). Then, if reference count is zero, it frees resource by calling cairo_surface_finish(). In other hands, cairo_surface_finish() drops resource to external resource directly. Could you let me know why cairo_surface_destroy() can fix memory leak ? cairo_surface_finish() doesn't free the surface. cairo_surface_destroy is the right function to destroy the surface when the RenderTheme is destroyed. As you already noticed that surface_destroy also calls the surface_finish when the reference count for a surface reaches zero, so we don't really need to call the surface_finish explicitly here. 625394 4 gyuyoung.kim 2012-05-16 07:20:39 -0700 rs+ on my side informally. However, I think rafael can review this patch as well. CC'ing rafael. 625443 5 antognolli+webkit 2012-05-16 08:06:51 -0700 (In reply to comment #4) > rs+ on my side informally. However, I think rafael can review this patch as well. CC'ing rafael. It also looks good to me. 625509 6 142226 webkit.review.bot 2012-05-16 09:00:08 -0700 Comment on attachment 142226 Fix memory leak Clearing flags on attachment: 142226 Committed r117297: <http://trac.webkit.org/changeset/117297> 625510 7 webkit.review.bot 2012-05-16 09:00:14 -0700 All reviewed patches have been landed. Closing bug. 142226 2012-05-16 05:29:03 -0700 2012-05-16 09:00:08 -0700 Fix memory leak 86609.patch text/plain 1298 naginenis ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA2MTE4MDZhLi5hZmM3MzU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTItMDUtMTYgIFN1ZGFyc2FuYSBOYWdpbmVuaSAgPHN1ZGFyc2FuYS5uYWdpbmVuaUBs aW51eC5pbnRlbC5jb20+CisKKyAgICAgICAgW0VGTF0gTWVtb3J5IGxlYWsgaW4gUmVuZGVyVGhl bWVFZmwKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg2 NjA5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4 IGEgbWVtb3J5IGxlYWsgYnkgZnJlZWluZyB0aGUgY2Fpcm8gc3VyZmFjZSB3aXRoCisgICAgICAg IGNhaXJvX3N1cmZhY2VfZGVzdHJveS4KKworICAgICAgICAqIHBsYXRmb3JtL2VmbC9SZW5kZXJU aGVtZUVmbC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUVmbDo6Y2FjaGVUaGVt ZVBhcnRGbHVzaCk6CisKIDIwMTItMDUtMTYgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5i b3RAZ21haWwuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxMTcwNTAu CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9lZmwvUmVuZGVyVGhlbWVFZmwu Y3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZWZsL1JlbmRlclRoZW1lRWZsLmNwcAppbmRl eCA4YjFiNzVjLi40NTg3OWQyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9l ZmwvUmVuZGVyVGhlbWVFZmwuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2VmbC9S ZW5kZXJUaGVtZUVmbC5jcHAKQEAgLTI3Niw3ICsyNzYsNyBAQCB2b2lkIFJlbmRlclRoZW1lRWZs OjpjYWNoZVRoZW1lUGFydEZsdXNoKCkKICAgICBlbmQgPSBtX3BhcnRDYWNoZS5lbmQoKTsKICAg ICBmb3IgKDsgaXRyICE9IGVuZDsgaXRyKyspIHsKICAgICAgICAgc3RydWN0IFRoZW1lUGFydENh Y2hlRW50cnkgKmVudHJ5ID0gKml0cjsKLSAgICAgICAgY2Fpcm9fc3VyZmFjZV9maW5pc2goZW50 cnktPnN1cmZhY2UpOworICAgICAgICBjYWlyb19zdXJmYWNlX2Rlc3Ryb3koZW50cnktPnN1cmZh Y2UpOwogICAgICAgICBldmFzX29iamVjdF9kZWwoZW50cnktPm8pOwogICAgICAgICBlY29yZV9l dmFzX2ZyZWUoZW50cnktPmVlKTsKICAgICAgICAgZGVsZXRlIGVudHJ5Owo=