--===============5597950037643652676== Content-Type: multipart/alternative; boundary="===============2679391636443088778==" --===============2679391636443088778== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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 --===============2679391636443088778== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123731/

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.
By Martin Klapetek.

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

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)

View Diff

--===============2679391636443088778==-- --===============5597950037643652676== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============5597950037643652676==--