Bug 87898

Summary:	Memory leak in recording_pattern_get_surface
Product:	cairo	Reporter:	Massimo <sixtysix>
Component:	xlib backend	Assignee:	Chris Wilson <chris>
Status:	RESOLVED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Massimo 2014-12-31 10:48:25 UTC

Running

cairo-test-suite -f record-extend-none-similar

valgrind reports:

>==1== 1,392 bytes in 3 blocks are definitely lost in loss record 1 of 7
>==1==    at 0x4A06BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>==1==    by 0x4C7042D: _cairo_recording_surface_snapshot (cairo/src/cairo-recording-surface.c:1427)
>==1==    by 0x4C842BE: _cairo_surface_snapshot_copy_on_write (cairo/src/cairo-surface-snapshot.c:189)
>==1==    by 0x4C7E7E0: _cairo_surface_detach_snapshot (cairo/src/cairo-surface.c:348)
>==1==    by 0x4C7E55B: _cairo_surface_detach_snapshots (cairo/src/cairo-surface.c:333)
>==1==    by 0x4C7E55B: _cairo_surface_flush (cairo/src/cairo-surface.c:1545)
>==1==    by 0x4C7E6CC: _cairo_surface_finish_snapshots (cairo/src/cairo-surface.c:1017)
>==1==    by 0x4C7E6CC: cairo_surface_destroy (cairo/src/cairo-surface.c:961)
>==1==    by 0x4C625A7: cairo_pattern_destroy (cairo/src/cairo-pattern.c:1131)
>==1==    by 0x4C3FAC6: _cairo_gstate_fini (cairo/src/cairo-gstate.c:225)
>==1==    by 0x4C3C68C: _cairo_default_context_fini (cairo/src/cairo-default-context.c:75)
>==1==    by 0x4C3C708: _cairo_default_context_destroy (cairo/src/cairo-default-context.c:93)
>==1==    by 0x43E576: record_get (cairo/test/record-extend.c:158)
>==1==    by 0x43E576: record_replay (cairo/test/record-extend.c:173)
>==1==    by 0x40E22D: cairo_test_for_target (cairo/test/cairo-test.c:929)
>==1==    by 0x40E22D: _cairo_test_context_run_for_target (cairo/test/cairo-test.c:1532)
>==1==    by 0x40B6C0: _cairo_test_runner_draw (cairo/test/cairo-test-runner.c:255)
>==1==    by 0x40B6C0: main (cairo/test/cairo-test-runner.c:937)

apparently  '_cairo_surface_snapshot_get_target' increases the target ref_count:

http://cgit.freedesktop.org/cairo/tree/src/cairo-surface-snapshot-inline.h#n55

so in 'recording_pattern_get_surface' in src/cairo-xlib-source.c it is necessary
to decrease it to match the behaviour of the 2 other possible code paths:
 
http://cgit.freedesktop.org/cairo/tree/src/cairo-xlib-source.c#n875

Comment 1 Chris Wilson 2014-12-31 11:04:18 UTC

commit 028d286e611d46755bb3d1e9932805de2ec35765
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Dec 31 10:54:43 2014 +0000

    xlib: Bump reference count for recording surface replays
    
    The snapshot takes a reference to the target recording surface in order
    to enable it for use by multiple treads. In order to balance this, the
    other two sources of recording surface must also take a reference and
    for us to release that reference after the replay.
    
    Otherwise, we end up with a memory leak:
    
    ==1== 1,392 bytes in 3 blocks are definitely lost in loss record 1 of 7
    ==1==    at 0x4A06BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==1==    by 0x4C7042D: _cairo_recording_surface_snapshot (cairo/src/cairo-recording-surface.c:1427)
    ==1==    by 0x4C842BE: _cairo_surface_snapshot_copy_on_write (cairo/src/cairo-surface-snapshot.c:189)
    ==1==    by 0x4C7E7E0: _cairo_surface_detach_snapshot (cairo/src/cairo-surface.c:348)
    ==1==    by 0x4C7E55B: _cairo_surface_detach_snapshots (cairo/src/cairo-surface.c:333)
    ==1==    by 0x4C7E55B: _cairo_surface_flush (cairo/src/cairo-surface.c:1545)
    ==1==    by 0x4C7E6CC: _cairo_surface_finish_snapshots (cairo/src/cairo-surface.c:1017)
    ==1==    by 0x4C7E6CC: cairo_surface_destroy (cairo/src/cairo-surface.c:961)
    ==1==    by 0x4C625A7: cairo_pattern_destroy (cairo/src/cairo-pattern.c:1131)
    ==1==    by 0x4C3FAC6: _cairo_gstate_fini (cairo/src/cairo-gstate.c:225)
    ==1==    by 0x4C3C68C: _cairo_default_context_fini (cairo/src/cairo-default-context.c:75)
    ==1==    by 0x4C3C708: _cairo_default_context_destroy (cairo/src/cairo-default-context.c:93)
    ==1==    by 0x43E576: record_get (cairo/test/record-extend.c:158)
    ==1==    by 0x43E576: record_replay (cairo/test/record-extend.c:173)
    ==1==    by 0x40E22D: cairo_test_for_target (cairo/test/cairo-test.c:929)
    ==1==    by 0x40E22D: _cairo_test_context_run_for_target (cairo/test/cairo-test.c:1532)
    ==1==    by 0x40B6C0: _cairo_test_runner_draw (cairo/test/cairo-test-runner.c:255)
    ==1==    by 0x40B6C0: main (cairo/test/cairo-test-runner.c:937)
    
    Reported-by: Massimo Valentini <mvalentini@src.gnome.org>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87898
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.