Leak involving Cairo under nsSVGGlyphFrame::SetupInheritablePaint

RESOLVED FIXED in mozilla21

Status

()

Core
SVG
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: njn, Assigned: jst)

Tracking

Trunk
mozilla21
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 707974 [details]
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.
(Assignee)

Comment 1

5 years ago
FWIW, the router I use is a netgear thingie with an installation of Tomato firmware, version 1.28.
(Assignee)

Updated

5 years ago
Attachment #707974 - Attachment mime type: text/x-log → text/plain
(Assignee)

Comment 2

5 years ago
Created attachment 708015 [details] [diff] [review]
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.
Attachment #708015 - Flags: review?(jwatt)
Comment on attachment 708015 [details] [diff] [review]
Likely fix.

Nice catch. r=me.
Attachment #708015 - Flags: review?(jwatt) → review+
(Reporter)

Updated

5 years ago
No longer blocks: 563700
Summary: Dark matter involving Cairo under nsSVGGlyphFrame::SetupInheritablePaint → Leak involving Cairo under nsSVGGlyphFrame::SetupInheritablePaint
(Assignee)

Comment 4

5 years ago
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 :(
(Assignee)

Updated

5 years ago
Assignee: nobody → jst
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cd7a9bc6406
(Reporter)

Comment 6

5 years ago
\o/
https://hg.mozilla.org/mozilla-central/rev/7cd7a9bc6406
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.