Bug 87898 - Memory leak in recording_pattern_get_surface
Summary: Memory leak in recording_pattern_get_surface
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: xlib backend (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Chris Wilson
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-31 10:48 UTC by Massimo
Modified: 2014-12-31 11:04 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Attachments

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.