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 680414 - Notification daemon appears to leak memory
Notification daemon appears to leak memory
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: message-tray
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-07-22 23:06 UTC by tombeckmann
Modified: 2013-01-02 17:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
notificationDaemon: Fix style (1.07 KB, patch)
2012-12-30 17:21 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
notificationDaemon: Prevent doing redundant work (1.06 KB, patch)
2012-12-30 17:21 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
notificationDaemon: Clean up code paths (2.03 KB, patch)
2012-12-30 17:21 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
messageTray: Support setImage(null) to mean unsetImage() (2.09 KB, patch)
2012-12-30 17:21 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
notificationDaemon: Merge two pieces of similar code (3.83 KB, patch)
2012-12-30 17:21 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
notificationDaemon: Clean up icon/image handling (5.23 KB, patch)
2012-12-30 17:22 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
notificationDaemon: Clean up icon/image handling (5.14 KB, patch)
2013-01-02 17:24 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description tombeckmann 2012-07-22 23:06:43 UTC
Sending a lot of notifications makes the used memory go up quite quickly, but it doesn't decrease later.
Comment 1 André Klapper 2012-07-23 07:58:00 UTC
Please provide a valgrind log and basic version and distribution information (gnome-shell, libnotify, etc).
Comment 2 Tony Houghton 2012-09-18 11:32:18 UTC
I'm also seeing a memory leak in GNOME, but I hadn't associated it with notifications. I'm willing to try running it in valgrind, but I'm not really sure how to do that. Is the easiest way to disable gdm3 and use startx with something like 'valgrind gnome-shell &>gnome-shell-valgrind.log' in .xinitrc?
Comment 3 Florian Müllner 2012-09-18 11:38:28 UTC
(In reply to comment #2)
> Is the easiest way to disable gdm3 and use startx with something like 
> 'valgrind gnome-shell &>gnome-shell-valgrind.log' in .xinitrc?

No - gnome-shell expects at least gnome-session and gnome-settings-daemon to be running as well. The easiest way is probably to do
  valgrind gnome-shell --replace
in your running session.
Comment 4 Tony Houghton 2012-09-19 18:08:02 UTC
That worked, thanks. Unfortunately the log file is huge, I'm uploading it to <http://www.realh.co.uk/misc/nohup.out>. I hope you can manage to sift some useful information out of it. If it would be more helpful for me to run it for a much shorter time, running fewer applications but forcing more notifications, please let me know.
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-12-30 17:21:19 UTC
Created attachment 232399 [details] [review]
notificationDaemon: Fix style
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-12-30 17:21:26 UTC
Created attachment 232400 [details] [review]
notificationDaemon: Prevent doing redundant work

We already calculated and created a gicon based on the icon and hints.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-12-30 17:21:32 UTC
Created attachment 232401 [details] [review]
notificationDaemon: Clean up code paths

While we really need to clean up the bad MessageTray API, this is
a quick step to allow for a cleaner codebase for the future.
Comment 8 Jasper St. Pierre (not reading bugmail) 2012-12-30 17:21:37 UTC
Created attachment 232402 [details] [review]
messageTray: Support setImage(null) to mean unsetImage()

This is a quick API change that should clean up some conditionals.
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-12-30 17:21:44 UTC
Created attachment 232403 [details] [review]
notificationDaemon: Merge two pieces of similar code

Use the same code for parsing notification data to handle both
icons and images.
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-12-30 17:22:51 UTC
Created attachment 232404 [details] [review]
notificationDaemon: Clean up icon/image handling

Make the logic for this clearer and easier to see.


--

This is a branch that I wrote this morning for unrelated reasons.
It should use less memory since it won't use the stex for notification
image pixbufs, which is a good idea.
Comment 11 Giovanni Campagna 2013-01-02 17:07:57 UTC
Review of attachment 232399 [details] [review]:

Yes.
Comment 12 Giovanni Campagna 2013-01-02 17:09:09 UTC
Review of attachment 232400 [details] [review]:

Yes.
Comment 13 Giovanni Campagna 2013-01-02 17:11:06 UTC
Review of attachment 232401 [details] [review]:

Ok
Comment 14 Giovanni Campagna 2013-01-02 17:12:17 UTC
Review of attachment 232402 [details] [review]:

Yes (although it would be nicer to avoid destroying the StBin every time)
Comment 15 Giovanni Campagna 2013-01-02 17:14:30 UTC
Review of attachment 232403 [details] [review]:

Ok
Comment 16 Giovanni Campagna 2013-01-02 17:16:54 UTC
Review of attachment 232404 [details] [review]:

::: js/ui/notificationDaemon.js
@@ +388,1 @@
         let image;

image = null here, or you're passing undefined if !gimage.
Comment 17 Jasper St. Pierre (not reading bugmail) 2013-01-02 17:24:49 UTC
Created attachment 232540 [details] [review]
notificationDaemon: Clean up icon/image handling

Make the logic for this clearer and easier to see.
Comment 18 Giovanni Campagna 2013-01-02 17:27:32 UTC
Comment on attachment 232540 [details] [review]
notificationDaemon: Clean up icon/image handling

Yes
Comment 19 Jasper St. Pierre (not reading bugmail) 2013-01-02 17:33:13 UTC
Attachment 232399 [details] pushed as fbc6292 - notificationDaemon: Fix style
Attachment 232400 [details] pushed as c11cbff - notificationDaemon: Prevent doing redundant work
Attachment 232401 [details] pushed as fe7ee1e - notificationDaemon: Clean up code paths
Attachment 232402 [details] pushed as 155f9dc - messageTray: Support setImage(null) to mean unsetImage()
Attachment 232403 [details] pushed as 1bd3494 - notificationDaemon: Merge two pieces of similar code
Attachment 232540 [details] pushed as bd38388 - notificationDaemon: Clean up icon/image handling