Last Comment Bug 180328 - Xft-related leaks
: Xft-related leaks
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Christopher Blizzard (:blizzard)
:
Mentors:
Depends on:
Blocks: xft_tracking
  Show dependency treegraph
 
Reported: 2002-11-15 09:09 PST by Christopher Blizzard (:blizzard)
Modified: 2011-11-15 17:19 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch to xftfreetype.c (1018 bytes, patch)
2003-01-19 07:52 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review

Description Christopher Blizzard (:blizzard) 2002-11-15 09:09:55 PST
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-7 (review requests must explain patch) 2002-12-05 16:56:32 PST
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 2002-12-05 17:07:45 PST
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) 2002-12-05 19:56:47 PST
Please do.  I'll try to get it into Red Hat's upstream sources.
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-01-19 07:52:16 PST
Created attachment 111972 [details] [diff] [review]
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 2003-01-19 10:26:18 PST
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-7 (review requests must explain patch) 2003-05-16 14:37:42 PDT
keithp: Any progress on looking at this patch?
Comment 7 keithp 2003-05-16 19:26:53 PDT
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) 2011-11-15 17:19:35 PST
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.

Note You need to log in before you can comment on or make changes to this bug.