[cairo] Memory leak with font variations

[Tobias Fleischer (reduxFX)]

 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
> that.
>
> 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
>> variations.
>> 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) :
>> NULL;
>>
>> 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);
>> cairo_font_options_destroy(t);
>> cairo_save(cr);
>> cairo_restore(cr);
>> cairo_destroy(cr);
>> cairo_surface_destroy(surface);
>>
>> I think what is missing is a matching free-and-null call in
>> _cairo_gstate_fini().
>> 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.
>> Cheers,
>> Toby
>> --
>> cairo mailing list
>> cairo at cairographics.org
>> https://lists.cairographics.org/mailman/listinfo/cairo
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.cairographics.org/archives/cairo/attachments/20200611/395bc9a4/attachment.htm>