Bug 333029 - memory leak in notifications
Summary: memory leak in notifications
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Notifications (show other bugs)
Version: master
Platform: unspecified Linux
: NOR normal
Target Milestone: 1.0
Assignee: Martin Klapetek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-03 15:27 UTC by David Edmundson
Modified: 2014-05-02 15:44 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Massif log (600.94 KB, application/octet-stream)
2014-04-03 15:27 UTC, David Edmundson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Edmundson 2014-04-03 15:27:04 UTC
Created attachment 85940 [details]
Massif log

Run this.

 while true ; do notify-send asdfadf; done

memory usage goes insane. 

Massif output implies problem is not a fault of notifications it's just that creates and deletes a /lot/ of dialogs.
Comment 1 Martin Klapetek 2014-04-04 08:52:59 UTC
> it's just that creates and deletes a /lot/ of dialogs.

That is true; each new notification gets new Dialog while after the notification is closed, the dialog is closed/destroyed.

I could possibly maintain a pool of 4 Dialogs and just reuse them (4 because 3 are on the screen at most while the 4th one would be the one sliding from top)
Comment 2 David Edmundson 2014-04-04 10:03:38 UTC
That seems like a bit like working round the problem and not actually fixing it.

Creating and deleting a dialog shouldn't use memory.
Comment 3 Martin Klapetek 2014-04-04 10:28:29 UTC
What happens is that for each notification received there's a new Dialog created, but not displayed, it's just queued for when there's an empty space on screen and only then shown.

Then it's deleteLater()'d so the memory is freed whenever it returns to the event loop.

How do you suggest to fix the problem then (and what is the problem)?

> Creating and deleting a dialog shouldn't use memory.

Correct. But wrong component for that :)
Comment 4 Marco Martin 2014-04-04 13:42:26 UTC
It's true that deleting a dialog shouldn't take memory (are you sure isn't just heap fragmentation?)

anyways, keeping a pool and reuse as much as possible looks something to do regardless to me
Comment 5 David Edmundson 2014-04-04 13:44:35 UTC
I don't think it's fragmentation as otherwise it wouldn't appear in massif which is tracking memory properly.
Comment 6 Martin Klapetek 2014-05-02 15:44:48 UTC
Git commit 8f76fed3a4cc3f1f16edc43d816c51f32ae05b88 by Martin Klapetek.
Committed on 02/05/2014 at 15:44.
Pushed by mklapetek into branch 'master'.

Fix memory leaking in notifications

Up to this patch there was a new Dialog created for each and every new
notification incoming. After the notification was timed out, it would
close and delete the Dialog, but somehow that didn't free all the memory
and over time plasma-shell could get huuuge mem usage.

Now the notifications reuse only 3 Dialogs the whole lifetime --> memory
does not skyrockets anymore.

Note that the original leak in Dialog might still be present, but
notifications do not expose it anymore.
REVIEW: 117903

M  +7    -4    applets/notifications/package/contents/ui/NotificationPopup.qml
M  +6    -5    applets/notifications/package/contents/ui/Notifications.qml
M  +60   -26   applets/notifications/plugin/notificationshelper.cpp
M  +16   -3    applets/notifications/plugin/notificationshelper.h

http://commits.kde.org/plasma-workspace/8f76fed3a4cc3f1f16edc43d816c51f32ae05b88