Review Request 123731: Cleanup handling of notifications closing

Martin Klapetek martin.klapetek at gmail.com
Mon May 25 08:23:40 UTC 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123731/
-----------------------------------------------------------

(Updated May 25, 2015, 8:23 a.m.)


Status
------

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
-------

Submitted with commit 35900a84a8e7de77747031c3ba26ed1ac61f389f by Martin Klapetek to branch master.


Repository: knotifications


Description
-------

While investigating the cause of crash of https://bugs.kde.org/show_bug.cgi?id=342752 I've come across some loose ends in how KNotifications is handling closing of notifications.

As for the crash itself, I never managed to reproduce even with special written test cases, but what happens (or seem to) is this:
 * KNotification object gets deleted
 * The destructor calls close() on all plugins
 * The NotifyByPopup plugin does async dbus call to close the notification and returns
 * KNotification's dpointer is deleted
 * The DBus reply returns, calling NotifyByPopup::finished()
 * Now for some reason the KNotification object is still not null but its dpointer is gone, so the check "if (!notification || d->notifications.contains(notification->id()))" crashes on the notification->id() call

So I've made it simply return -1 when dpointer is null, which should ideally prevent the crashes


Diffs
-----

  src/knotification.cpp 01962b3 
  src/knotificationmanager.cpp 0d9b3b0 
  src/knotificationmanager_p.h f8e7df8 
  src/notifybypopup.cpp 2f84ab0 

Diff: https://git.reviewboard.kde.org/r/123731/diff/


Testing
-------


Thanks,

Martin Klapetek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150525/3af33255/attachment-0001.html>


More information about the Plasma-devel mailing list