Differential D15623

Finish the notification before removing it from the hash
AbandonedPublic
Actions

Authored by jtamate on Sep 20 2018, 2:41 PM.

Details

Reviewers
sitter
Group Reviewers
Frameworks
Summary

If the notification is deleted from the hash, when the notification::id is called later by finish, it could contain anything.
Change the order, call finish with a valid notification pointer before removing the notification from the hash.

BUG: 398695

Diff Detail

Repository
R289 KNotifications
Lint
Lint Skipped
Unit
Unit Tests Skipped
jtamate created this revision.Sep 20 2018, 2:41 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 20 2018, 2:41 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
jtamate requested review of this revision.Sep 20 2018, 2:41 PM
sitter added a comment.Sep 21 2018, 7:42 AM

I am not sure I understand this. (admittedly I hadn't had enough coffee yet though)

NotifyByAudio doesn't own the KNotification *, it gets it form the outside via ::notify(). Why would removing the entry from the internal "cache" hash m_notifications cause its deletion?

jtamate abandoned this revision.Sep 21 2018, 7:51 AM

I didn't realize the notifications are not owned by m_notifications :-(.
Just looking at the backtrace of the crash, it was the only suspicious thing.

sitter added a comment.Sep 21 2018, 7:53 AM

Don't fret, it was a good idea :)

Revision Contents
Changeset List

PathPackages
Msrc/notifybyaudio_canberra.cpp (4 lines)

Diff 41992

View Options

src/notifybyaudio_canberra.cpp

Loading...