> On Oct. 11, 2016, 3:16 p.m., Anthony Fieroni wrote: > > src/knotificationmanager.cpp, line 203 > > <https://git.reviewboard.kde.org/r/129146/diff/1/?file=481937#file481937line203> > > > > n->close() > > David Edmundson wrote: > I did think about that, but what if the userspace code is: > > KNotification *n = new KNoti.. > n->ref(); > n->sendEvent(); > > we wouldn't want to close immediately.
User must *never* do that, ref/deref interface is only for manager. - Anthony ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129146/#review99930 ----------------------------------------------------------- On Oct. 11, 2016, 2:19 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129146/ > ----------------------------------------------------------- > > (Updated Oct. 11, 2016, 2:19 p.m.) > > > Review request for KDE Frameworks. > > > Repository: knotifications > > > Description > ------- > > KNotificatitionManager::self() has an early check if a notification has > no actions. If it has no actions it tries to close the notification > early. However it's dereferencing an object that it has not referenced, > sending the ref count to -1, a corrupt state that is not handled > gracefully in KNotification, and we don't ever close the notification. > > By referencing and dereferencing we're still calling the cleanup in > KNotification (if applicable) but without sending the ref count negative. > > This fixes ksmserver waiting for the logout sound to play; which > currently never emits close, as the default setup has no logout sound. > > > Diffs > ----- > > autotests/knotification_test.cpp 5d063f1a19d7f5e4d8c77d7e6301e81223401b08 > src/knotificationmanager.cpp c315db9a3f17771d4095cfdc7982475949213a1c > > Diff: https://git.reviewboard.kde.org/r/129146/diff/ > > > Testing > ------- > > Autotest that used to fail included > > > Thanks, > > David Edmundson > >
