[cairo] Memory leak with image surfaces

[Victor Goya]

Git bisect points to this commit:

commit 0bfd2acd35547fc2bd0de99cc67d153f0170697d
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Mon Aug 13 01:34:12 2012 +0100

    xlib: Implement SHM fallbacks and fast upload paths

This commit is rather big, I can't figure out what is wrong with it yet.

-- 
Victor Goya

On Fri, Jun 14, 2013 at 8:15 PM, Victor Goya <victor.goya at af83.com> wrote:
> Hello!
>
> The following test produces a memory leak in the read_png() method:
>
> #include <cairo/cairo.h>
> #include <stdlib.h>
>
> int main() {
>     int              w, h;
>     cairo_surface_t *image;
>     cairo_surface_t *pdf;
>     cairo_t *        cr;
>     int i;
>
>     pdf = cairo_pdf_surface_create("/tmp/lol.pdf", 1500, 1500);
>     cr = cairo_create(pdf);
>
>     image = cairo_image_surface_create_from_png ("./test.png");
>     w = cairo_image_surface_get_width (image);
>     h = cairo_image_surface_get_height (image);
>
>     cairo_scale (cr, 256.0/w, 256.0/h);
>
>     cairo_set_source_surface (cr, image, 0, 0);
>     cairo_paint (cr);
>     cairo_surface_destroy (image);
>
>     cairo_show_page(cr);
>
>     cairo_surface_destroy(pdf);
>     cairo_destroy (cr);
> }
>
> Valgrind output :
>
> ==29797== HEAP SUMMARY:
> ==29797==     in use at exit: 172,738 bytes in 13 blocks
> ==29797==   total heap usage: 84 allocs, 71 frees, 997,686 bytes allocated
> ==29797==
> ==29797== 160,000 bytes in 1 blocks are definitely lost in loss record 13 of 13
> ==29797==    at 0x4C28BED: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==29797==    by 0x4EDC7D1: read_png (cairo-png.c:661)
> ==29797==    by 0x4EDD1ED: cairo_image_surface_create_from_png (cairo-png.c:779)
> ==29797==    by 0x400AAC: main (test.c:14)
> ==29797==
> ==29797== LEAK SUMMARY:
> ==29797==    definitely lost: 160,000 bytes in 1 blocks
> ==29797==    indirectly lost: 0 bytes in 0 blocks
> ==29797==      possibly lost: 0 bytes in 0 blocks
> ==29797==    still reachable: 12,738 bytes in 12 blocks
> ==29797==         suppressed: 0 bytes in 0 blocks
>
> I ran into the same problem with larger PDF files, the decompressed
> images are never freed.
>
> The issue seems to be related to the "COW snapshots" feature :
> "_cairo_image_surface_snapshot" create a clone of the surface but set
> the "owns_data" flag of both the surface and its clone to FALSE. This
> can lead in a situation where neither the original surface nor its
> clone own the data, this results in a memory leak because none of them
> take the responsability to free it.
>
> My guess is that the clone should copy the "owns_data" flag of the
> original surface before setting the "owns_data" flag of the original
> surface to FALSE. This way the clone is responsible to free the data
> only if the original surface owned it. I'm not quite sure about all
> the implications of the attached patch, but it fixes the leak for my
> use case.
>
> --- a/src/cairo-image-surface.c
> +++ b/src/cairo-image-surface.c
> @@ -762,12 +762,13 @@ _cairo_image_surface_snapshot (void *abstract_surface)
>      return &clone->base;
>
>   image->pixman_image = NULL;
> +
> + clone->owns_data = image->owns_data;
>   image->owns_data = FALSE;
>
>   clone->transparency = image->transparency;
>   clone->color = image->color;
>
> - clone->owns_data = FALSE;
>   return &clone->base;
> }
>
> Thanks!
>
> --
> Victor Goya