836173 - Leak involving Cairo under nsSVGGlyphFrame::SetupInheritablePaint

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: DMD output

jst used DMD on a long-running session.  It crashed while producing the output (see bug 836054) but spat out the top three unreported records before doing so, which together accounted for 12% of his heap.  The full output is attached.

The first and third record accounted for 66 MB and looked like this:

>   replace_malloc (/home/jst/fast/work/tip/mozilla/memory/replace/dmd/DMD.cpp:1222) 0x7f9e6360c1a8
>   _cairo_pattern_create_solid (/home/jst/fast/work/tip/mozilla/gfx/cairo/cairo/src/cairo-pattern.c:490) 0x7f9e5f24b6ec
>   *INT__moz_cairo_pattern_create_rgba (/home/jst/fast/work/tip/mozilla/gfx/cairo/cairo/src/cairo-pattern.c:593) 0x7f9e5f24b857
>   gfxPattern (/home/jst/fast/work/tip/mozilla/gfx/thebes/gfxPattern.cpp:26) 0x7f9e5f188747
>   nsSVGGlyphFrame::SetupInheritablePaint(gfxContext*, float&, gfxTextObjectPaint*, nsSVGGlyphFrame::SVGTextObjectPaint::Paint&, nsStyleSVGPaint nsStyle
SVG::*, mozilla::FramePropertyDescriptor const*) (asn1cmn.c:0) 0x7f9e5eb8855d
>   nsSVGGlyphFrame::SetupCairoFill(gfxContext*, gfxTextObjectPaint*, nsSVGGlyphFrame::SVGTextObjectPaint*) (asn1cmn.c:0) 0x7f9e5eb88615
>   nsSVGGlyphFrame::SetupCairoState(gfxContext*, gfxTextObjectPaint*, gfxTextObjectPaint**) (asn1cmn.c:0) 0x7f9e5eb89935
>   nsSVGGlyphFrame::PaintSVG(nsRenderingContext*, nsIntRect const*) (asn1cmn.c:0) 0x7f9e5eb8b562

(I think the filenames and line numbers aren't right here, but hopefully the function names are.)  

Unfortunately, the memory is allocated within Cairo, and we don't currently have any memory reporting for Cairo.

The second record accounted for 45 MB and looked like this:

>   replace_malloc (/home/jst/fast/work/tip/mozilla/memory/replace/dmd/DMD.cpp:1222) 0x7f9e6360c1a8
>   moz_xmalloc (/home/jst/fast/work/tip/mozilla/memory/mozalloc/mozalloc.cpp:55) 0x7f9e60c94e5a
>   nsSVGGlyphFrame::SetupInheritablePaint(gfxContext*, float&, gfxTextObjectPaint*, nsSVGGlyphFrame::SVGTextObjectPaint::Paint&, nsStyleSVGPaint nsStyle
SVG::*, mozilla::FramePropertyDescriptor const*) (asn1cmn.c:0) 0x7f9e5eb8854e
>   nsSVGGlyphFrame::SetupCairoFill(gfxContext*, gfxTextObjectPaint*, nsSVGGlyphFrame::SVGTextObjectPaint*) (asn1cmn.c:0) 0x7f9e5eb88615
>   nsSVGGlyphFrame::SetupCairoState(gfxContext*, gfxTextObjectPaint*, gfxTextObjectPaint**) (asn1cmn.c:0) 0x7f9e5eb89935
>   nsSVGGlyphFrame::PaintSVG(nsRenderingContext*, nsIntRect const*) (asn1cmn.c:0) 0x7f9e5eb8b562
>   nsDisplaySVGGlyphs::Paint(nsDisplayListBuilder*, nsRenderingContext*) (asn1cmn.c:0) 0x7f9e5eb887bd

I looks rather similar, and may be basically the same thing.

jst said his workload involves a page that uses SVG:  "it's an internal interface to my router, so not something that is publicly accessible. I always have a firefox window open that shows bandwidth graph on the router, and that graph is done with a live SVG image. That's more than likely the culprit here if my above assumptions are correct, and that graph updates once every 5 seconds or so, so lots of changes over time."  This was a very long-running session, days or even weeks.

Comment 1

[Johnny Stenback (:jst)]

FWIW, the router I use is a netgear thingie with an installation of Tomato firmware, version 1.28.

Comment 2

[Johnny Stenback (:jst)]

Attachment: Likely fix.

If my quick look here based on what Nick found is correct then this might not be a memory reporter problem at all, but an actual leak.

If cairo is used, as I suspect is the case for me (because vnc), then this code passes in an unowned  gfxPattern pointer to gfxContext::SetPattern(), which in that case just extracts the cairo pattern from it and returns, w/o anything actually holding on to or ever releasing the given gfxPattern. That leaves the gfxPattern hanging w/o anything referencing it.

This compiles and seems to run, but I have not verified that this fixes this problem.

Comment 3

[Jonathan Watt [:jwatt]]

Comment on attachment 708015 [details] [diff] [review]
Likely fix.

Nice catch. r=me.

Comment 4

[Johnny Stenback (:jst)]

Tested that patch, and it does indeed fix a leak! W/o that patch, the number of allocated gfxPattern's is ever growing, with the patch, we create ~14 and stay there. As a side note, we also appear to leak those 14 or so :(

Comment 5

[Johnny Stenback (:jst)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7cd7a9bc6406

Comment 6

[Nicholas Nethercote [inactive]]

\o/

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7cd7a9bc6406