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 709264 - Fix memory leaks in icons handling
Fix memory leaks in icons handling
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 709243
 
 
Reported: 2013-10-02 13:38 UTC by Bastien Nocera
Modified: 2013-10-02 15:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
icon-theme: Use g_clear_* helpers in finalize (1.32 KB, patch)
2013-10-02 13:38 UTC, Bastien Nocera
committed Details | Review
icon-theme: Fix 2 memory leaks in GtkIconInfo (950 bytes, patch)
2013-10-02 13:38 UTC, Bastien Nocera
committed Details | Review
iconhelper: Use g_clear_* helpers (1.61 KB, patch)
2013-10-02 13:38 UTC, Bastien Nocera
committed Details | Review
iconhelper: Fix leak when rendering to cairo surface (812 bytes, patch)
2013-10-02 13:38 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2013-10-02 13:38:27 UTC
The background panel makes heavy use of loadable icons, and this causes
leaks as big as each of the icons each time they're rendered.
Comment 1 Bastien Nocera 2013-10-02 13:38:41 UTC
Created attachment 256262 [details] [review]
icon-theme: Use g_clear_* helpers in finalize
Comment 2 Bastien Nocera 2013-10-02 13:38:46 UTC
Created attachment 256263 [details] [review]
icon-theme: Fix 2 memory leaks in GtkIconInfo
Comment 3 Bastien Nocera 2013-10-02 13:38:51 UTC
Created attachment 256264 [details] [review]
iconhelper: Use g_clear_* helpers
Comment 4 Bastien Nocera 2013-10-02 13:38:56 UTC
Created attachment 256265 [details] [review]
iconhelper: Fix leak when rendering to cairo surface
Comment 5 Emmanuele Bassi (:ebassi) 2013-10-02 14:43:54 UTC
Review of attachment 256262 [details] [review]:

looks good
Comment 6 Emmanuele Bassi (:ebassi) 2013-10-02 14:44:26 UTC
Review of attachment 256263 [details] [review]:

looks good.
Comment 7 Emmanuele Bassi (:ebassi) 2013-10-02 14:46:05 UTC
Review of attachment 256264 [details] [review]:

this is not really needed, except for dropping some code. on the other hand, it's a finalize() implementation.
Comment 8 Emmanuele Bassi (:ebassi) 2013-10-02 14:47:17 UTC
Review of attachment 256265 [details] [review]:

looks good.
Comment 9 Emmanuele Bassi (:ebassi) 2013-10-02 14:49:55 UTC
in general, I think we need to clear up that g_clear_object/pointer are needed not for reducing lines of code, but for atomic access to clear a pointer. GTK+ does not really need that, as we assume that all GTK+ code is called from the same thread (stuff will blow up if that's not the case, so a leak would be the least of our worries), and atomic operations are not exactly cheap. on the other hand, we're already using g_clear_* everywhere, so it's not like it's going to be worse than what we already have.
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-10-02 15:14:18 UTC
(In reply to comment #9)
> in general, I think we need to clear up that g_clear_object/pointer are needed
> not for reducing lines of code, but for atomic access to clear a pointer.

I use it to reduce lines of code, because it's super convenient. We're really not going to gain speedups by dropping the atomics -- the free(); is 20 times more expensive than atomics. Allocate smarter if you want to speed up your code.
Comment 11 Bastien Nocera 2013-10-02 15:22:44 UTC
Pushed the icon-theme fixes to gtk-3-8, and all the fixes to gtk-3-10 and master.

Attachment 256262 [details] pushed as 7c595bc - icon-theme: Use g_clear_* helpers in finalize
Attachment 256263 [details] pushed as 8938f3c - icon-theme: Fix 2 memory leaks in GtkIconInfo
Attachment 256264 [details] pushed as e53bb1b - iconhelper: Use g_clear_* helpers
Attachment 256265 [details] pushed as 744b790 - iconhelper: Fix leak when rendering to cairo surface