180328 - Xft-related leaks

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Christopher Blizzard (:blizzard)]

Mail from dbaron:

The leak tools are showing a lot of leaks from a single stack:

FcPatternCreate[/usr/lib/libfontconfig.so.1 +0x14763]
FcFontRenderPrepare[/usr/lib/libfontconfig.so.1 +0x12BA9]
nsFontXft::GetXftFont()[/builds/trunk/obj/debug/dist/bin/components/libgfx_gtk.so
+0x43D7C]

In both these places, we pass ownership of this pattern to the font
through XftFontOpenPattern.  Looking at xftfreetype.c in the source RPM,
it seems like this function is designed to take ownership of the pattern
since it calls FcPatternDestroy on the argument in a case where it
doesn't need the pattern.  However, I can't find the code in the normal
case that destroys the pattern when destroying the font.  Does this seem
like an Xft bug to you?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I believe this is unrelated to
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=76219 , but I'll mention it
here anyway.

keithp, any thoughts on whether this is a real bug?

Comment 2

[keithp]

Sure looks like I goofed on this one.  Both the pattern and the charset are
never freed (note that the charset is generally shared with the cached font
info, so that's not noticed as a leak).

This problem is mitigated by the sharing of identical fonts and the caching of
recently closed fonts; in both of those cases the pattern is correctly freed.

I'll build a test infrastructure and see if I can't verify the bug and then add
the obvious fix.

Comment 3

[Christopher Blizzard (:blizzard)]

Please do.  I'll try to get it into Red Hat's upstream sources.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch to xftfreetype.c

This patch fixes the leaks for me.

I never actually observed the charset leak, but that looks like it also leaks
(in the case where it calls FcFreeTypeCharSet, which seems like it might be an
edge case) since FcFreeTypeCharSet returns the result of FcCharSetCreate.

I've been running with this for about 10 minutes so far, but I'll continue to
run with it.

Keith, does this patch seem right?  (Or did you already patch it?  I don't see
a fix in
http://cvsweb.xfree86.org/cvsweb/~checkout~/xc/lib/Xft/xftfreetype.c?rev=1.29&content-type=text/plain

.)

Comment 5

[keithp]

Yes, the patch looks right.  I'm a bit surprised that you're hitting it as it
should only occur for fonts which aren't found in the fontconfig cache.

I'm afraid I won't get a chance to apply this patch for a couple of weeks as it
will require some regression testing.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

keithp: Any progress on looking at this patch?

Comment 7

[keithp]

Thanks for the reminder.  I've committed a fixed version of the provided patch
to Xft CVS (at cvs.fontconfig.org).  I also found a leak in fontconfig at the
same time and fixed that too.

Comment 8

[Zack Weinberg (:zwol)]

This appears to have been a bug not actually in Mozilla but in fontconfig, and to have been fixed many years ago.  And I'm not sure what it was doing in the XUL component.