After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 649497 - st-private: Fix memory leak
st-private: Fix memory leak
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-05-05 19:22 UTC by Colin Walters
Modified: 2011-05-11 16:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
st-private: Fix memory leak (1.48 KB, patch)
2011-05-05 19:22 UTC, Colin Walters
committed Details | Review
st-private: Correct fix for memory leak (2.00 KB, patch)
2011-05-11 15:23 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2011-05-05 19:22:48 UTC
==13810== 11,360 bytes in 1 blocks are definitely lost in loss record 18,574 of 18,765
==13810==    at 0x4005447: calloc (vg_replace_malloc.c:467)
==13810==    by 0x5191882: standard_calloc (gmem.c:107)
==13810==    by 0x51920A7: g_malloc0 (gmem.c:196)
==13810==    by 0x4056201: blur_pixels (st-private.c:466)
==13810==    by 0x40573B4: _st_create_shadow_cairo_pattern (st-private.c:710)
==13810==    by 0x4070746: st_theme_node_paint (st-theme-node-drawing.c:856)
==13810==    by 0x3FEFFFFF: ???
Comment 1 Colin Walters 2011-05-05 19:22:50 UTC
Created attachment 187312 [details] [review]
st-private: Fix memory leak
Comment 2 Colin Walters 2011-05-05 20:01:25 UTC
Attachment 187312 [details] pushed as 72f9f48 - st-private: Fix memory leak
Comment 3 Florian Müllner 2011-05-05 20:06:15 UTC
Review of attachment 187312 [details] [review]:

::: src/st/st-private.c
@@ +786,3 @@
 
+ out:
+  g_free (pixels_out);

Why not free pixels_out right after surface_out is destroyed? Obviously goto is not evil here, but it's a bit odd that some data is freed along the way and other is kept around until the end ...
Comment 4 Florian Müllner 2011-05-05 20:07:36 UTC
Comment on attachment 187312 [details] [review]
st-private: Fix memory leak

Woops, too late :-)
Comment 5 Colin Walters 2011-05-05 21:12:48 UTC
Actually it's not that easy; cairo is going to be using the data as long as the pattern is alive.  Now I get:

==22480== Invalid read of size 4
==22480==    at 0x509203C: sse2_composite_add_8000_8000 (pixman-sse2.c:587)
==22480==    by 0x50767FE: pixman_image_composite32 (pixman.c:397)
==22480==    by 0x4FA8D13: _composite_mask (cairo-image-surface.c:3318)
==22480==    by 0x4FAAA79: _create_composite_mask_pattern (cairo-image-surface.c:1931)
==22480==    by 0x4FAAD06: _clip_and_composite (cairo-image-surface.c:1980)
==22480==    by 0x4FAD21C: _cairo_image_surface_mask (cairo-image-surface.c:3364)
==22480==    by 0xFF7FFFFF: ???
==22480==  Address 0x74584d9 is 857 bytes inside a block of size 11,360 free'd
==22480==    at 0x4005ECD: free (vg_replace_malloc.c:366)
==22480==    by 0x5192FEA: standard_free (gmem.c:101)
==22480==    by 0x51931C5: g_free (gmem.c:263)
==22480==    by 0x4057481: _st_create_shadow_cairo_pattern (st-private.c:788)
==22480==    by 0x4070756: st_theme_node_paint (st-theme-node-drawing.c:856)
==22480==    by 0x3FEFFFFF: ???
Comment 6 Colin Walters 2011-05-11 15:23:14 UTC
Created attachment 187638 [details] [review]
st-private: Correct fix for memory leak

The previous fix in 72f9f482d was wrong; we need to keep around
the buffer until cairo is done with the pattern.
Comment 7 Florian Müllner 2011-05-11 15:37:33 UTC
Review of attachment 187638 [details] [review]:

Looks good.
Comment 8 Colin Walters 2011-05-11 16:01:22 UTC
Attachment 187638 [details] pushed as c975740 - st-private: Correct fix for memory leak