874768 - Ensure that Canvas2D on Skia/GL runs without Valgrind memcheck errors

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Goal of this bug: on any platform where Valgrind runs, get a full run of Canvas 2D tests on Skia/GL without memcheck errors.

Current status: with the latest rebase (bug 848491) and the pthreads patch (bug 874682), on Linux x86-64, 64bit build, I can get a full run of

  http://philip.html5.org/tests/canvas/suite/tests/index.all.html

in Valgrind, but when I quit this browser, I eventually get Valgrind bailing out and killing it before the end, with this message:

--22932:0:aspacem  Valgrind: FATAL: VG_N_SEGMENTS is too low.
--22932:0:aspacem    Increase it and rebuild.  Exiting now.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Alright, I could carry by tweaking these magic values in Valgrind:

bjacob:/hack/valgrind$ svn diff
Index: coregrind/m_aspacemgr/aspacemgr-linux.c
===================================================================
--- coregrind/m_aspacemgr/aspacemgr-linux.c     (revision 13401)
+++ coregrind/m_aspacemgr/aspacemgr-linux.c     (working copy)
@@ -265,10 +265,10 @@
 /* ------ start of STATE for the address-space manager ------ */
 
 /* Max number of segments we can track. */
-#define VG_N_SEGMENTS 5000
+#define VG_N_SEGMENTS 50000
 
 /* Max number of segment file names we can track. */
-#define VG_N_SEGNAMES 1000
+#define VG_N_SEGNAMES 10000
 
 /* Max length of a segment file name. */
 #define VG_MAX_SEGNAMELEN 1000

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: remaining leaks according to memcheck on June 17 (graphics branch, cset debcc24281dc, after ownership-refactoring and other misc fixes)

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: remaining leaks after bug 886518 patch

What's left:

- profiler pseudostacks leak (the threads stuff) already fixed in mozilla-central --- we're only hitting this because the last mozilla-central->graphics merge was a long time ago

- an interesting pango-related leak:

==18492== 1,152 bytes in 18 blocks are definitely lost in loss record 7,058 of 7,255
==18492==    at 0x402C7C1: malloc (vg_replace_malloc.c:292)
==18492==    by 0xC7FFEBE: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.4.4)
==18492==    by 0xC8003E9: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.4.4)
==18492==    by 0xC805D58: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.4.4)
==18492==    by 0xC80628F: FcFreeTypeCharSetAndSpacing (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.4.4)
==18492==    by 0xC8077C9: FcFreeTypeQueryFace (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.4.4)
==18492==    by 0x84B8ED5: gfxDownloadedFcFontEntry::InitPattern() (gfxPangoFonts.cpp:548)
==18492==    by 0x84BBEAC: gfxPangoFontGroup::NewFontEntry(gfxProxyFontEntry const&, unsigned char const*, unsigned int) (gfxPangoFonts.cpp:1790)
==18492==    by 0x84F0B08: gfxUserFontSet::LoadFont(gfxMixedFontFamily*, gfxProxyFontEntry*, unsigned char const*, unsigned int&) (gfxUserFontSet.cpp:646)
==18492==    by 0x84F1504: gfxUserFontSet::OnLoadComplete(gfxMixedFontFamily*, gfxProxyFontEntry*, unsigned char const*, unsigned int, tag_nsresult) (gfxUserFontSet.cpp:421)
==18492==    by 0x7382D92: nsFontFaceLoader::OnStreamComplete(nsIStreamLoader*, nsISupports*, tag_nsresult, unsigned int, unsigned char const*) (nsFontFaceLoader.cpp:217)
==18492==    by 0x6F9F47B: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (nsStreamLoader.cpp:101)
==18492== 


- the SkGlyphCache is leaked, but that is intentional according to this comment:
http://hg.mozilla.org/mozilla-central/file/c87a950e1a09/gfx/skia/src/core/SkGlyphCache.cpp#l491

==18492== 648 bytes in 27 blocks are definitely lost in loss record 6,943 of 7,255
==18492==    at 0x402C7C1: malloc (vg_replace_malloc.c:292)
==18492==    by 0x85D1223: _moz_cairo_font_options_create (cairo-font-options.c:107)
==18492==    by 0x87ACBCA: SkScalerContext_CairoFT::SkScalerContext_CairoFT(SkTypeface*, SkDescriptor const*) (SkFontHost_cairo.cpp:188)
==18492==    by 0x87ACD1E: SkCairoFTTypeface::onCreateScalerContext(SkDescriptor const*) const (SkFontHost_cairo.cpp:94)
==18492==    by 0x887B38E: SkTypeface::createScalerContext(SkDescriptor const*) const (SkScalerContext.cpp:787)
==18492==    by 0x884BCE9: SkGlyphCache::SkGlyphCache(SkTypeface*, SkDescriptor const*) (SkGlyphCache.cpp:64)
==18492==    by 0x884BE2F: SkGlyphCache::VisitCache(SkTypeface*, SkDescriptor const*, bool (*)(SkGlyphCache const*, void*), void*) (SkGlyphCache.cpp:569)
==18492==    by 0x8860E81: DetachDescProc(SkTypeface*, SkDescriptor const*, void*) (SkPaint.cpp:419)
==18492==    by 0x8862E1C: SkPaint::descriptorProc(SkDeviceProperties const*, SkMatrix const*, void (*)(SkTypeface*, SkDescriptor const*, void*), void*, bool) const (SkPaint.cpp:1878)
==18492==    by 0x8862E7A: SkPaint::detachCache(SkDeviceProperties const*, SkMatrix const*) const (SkPaint.cpp:1884)
==18492==    by 0x88421D8: SkAutoGlyphCache::SkAutoGlyphCache(SkPaint const&, SkDeviceProperties const*, SkMatrix const*) (SkGlyphCache.h:283)
==18492==    by 0x8842902: SkDraw::drawPosText(char const*, unsigned long, float const*, float, int, SkPaint const&) const (SkDraw.cpp:1851)


- the rest seems to be noise, mostly GTK-related.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: remaining leaks after updated bug 886518 patch

Hm, the pango leak is not seen in this log. Checking if it makes sense that this small change would have fixed it.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: remaining leaks after updated bug 886518 patch, entire 2D canvas tests run

....here's a valgrind leak log for a run of the entire canvas test suite, confirming that the gfxPangoFonts leak doesn't reproduce anymore. With the SkGlyphCache leak being understood as intentional (see above) and the mozilla_sampler_register_thread being a known leak in the Gecko profiler, already fixed upstream, we can consider ourselves leak-free now!