Bug 86609 – [EFL] Memory leak in RenderThemeEfl

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Sudarsana Nagineni (babu)]

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)

[Sudarsana Nagineni (babu)]

Created attachment 142226 [details]
Fix memory leak

Free the surface with cairo_surface_destroy.

[Gyuyoung Kim]

Comment on attachment 142226 [details]
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 ?

[Sudarsana Nagineni (babu)]

(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.

[Gyuyoung Kim]

rs+ on my side informally. However, I think rafael can review this patch as well. CC'ing rafael.

[Rafael Antognolli]

(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.

[WebKit Review Bot]

Comment on attachment 142226 [details]
Fix memory leak

Clearing flags on attachment: 142226

Committed r117297: <http://trac.webkit.org/changeset/117297>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.