[cairo] Memory leak with font variations
behdad at behdad.org
Sat Jun 13 20:44:15 UTC 2020
You are absolutely right. It's a mess.
cairo_font_options_t used to be a plain old struct that didn't own
anything. The ->get_font_options() unfortunately was designed to take a
*options that it assumed didn't need cleanup and just plain wrote to it.
When we added variations, all those assumptions broke. I've given it a try
to clean up:
I think I got it all right but am not completely sure. I think what I did
is the best approach given the mess we are in.
Please review and test. It compiles but I haven't tested at all. I also
wrote the patch on a very old checkout of mine then rebase on master. I
missed any code in between that would need the same treatment but can't do
another review right now.
Hope someone can take it from there.
PS. I also found this mirror is stalled and confusing:
On Wed, Jun 10, 2020 at 10:39 PM Tobias Fleischer (reduxFX) <
tobias.fleischer at reduxfx.com> wrote:
> Unfortunately, I just found out it is not the only location where this is
> happening :(
> For example cairo_scaled_font_create() also calls
> _cairo_font_options_init_copy(), which then leads to a leak.
> I tried to add these lines to _cairo_scaled_font_fini_internal():
> _cairo_font_options_fini (&scaled_font->options);
> scaled_font->options.variations = NULL;
> But that didn't help, I still get leaks because of unmatched malloc/free
> blocks. Maybe it has to do with the fontmap caching/release count here?
> Or maybe we shouldn't try to duplicate the variation option string at all,
> but just copy the pointer and say it is up to the calling application to
> guarantee its existence?
> A fixed char array could also be an option but that does sound very hacky.
> On Thu, Jun 11, 2020 at 7:30 AM Behdad Esfahbod <behdad at behdad.org> wrote:
>> Thanks Tobias. This definitely makes sense. Thanks for debugging. I'll
>> try to take a look on computer and fix. Unless, I hope, someone beats me to
>> On Wed, Jun 10, 2020, 10:25 PM Tobias Fleischer (reduxFX) <
>> tobias.fleischer at reduxfx.com> wrote:
>>> I think I found a bug concerning non-released memory when using font
>>> I tested against cairo-1.17.2.
>>> The internal function _cairo_gstate_init_copy() is supposed to make a
>>> deep copy of the fields from one instance/state to another, used for
>>> example by cairo_save(). It does however call
>>> _cairo_font_options_init_copy(), which has this line in it:
>>> options->variations = other->variations ? strdup (other->variations) :
>>> This means that if a font variation string has been set, instead of a
>>> copy, it will always allocate and use a copy of the string (via strdup),
>>> which will then never be freed.
>>> This leads to memory leaks as for example just by calling cairo_save(),
>>> with each call an additional pointer is created that is never released.
>>> Simple sample code to reproduce:
>>> cairo_surface_t* surface =
>>> cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 1920, 1080);
>>> cairo_t* cr = cairo_create(surface);
>>> cairo_font_options_t* t = cairo_font_options_create();
>>> cairo_get_font_options(cr, t);
>>> cairo_font_options_set_variations(t, "wght=400");
>>> cairo_set_font_options(cr, t);
>>> I think what is missing is a matching free-and-null call in
>>> If I add the following two lines at the beginning
>>> of_cairo_gstate_fini(), it seems to fix this issue, as every allocated copy
>>> gets freed again:
>>> _cairo_font_options_fini (&gstate->font_options);
>>> gstate->font_options.variations = NULL;
>>> Let me know if this makes sense.
>>> cairo mailing list
>>> cairo at cairographics.org
> cairo mailing list
> cairo at cairographics.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cairo