Review Request 121885: Properly check for systray being available

[Martin Gräßlin]

> On Jan. 6, 2015, 7:20 p.m., David Edmundson wrote:
> > src/platformtheme/kdeplatformsystemtrayicon.cpp, line 330
> > <https://git.reviewboard.kde.org/r/121885/diff/1/?file=338653#file338653line330>
> >
> >     the statusnotifierwatcher is a kded module, which means if another kded module tries to create an SNI (and at least one does) we're going to deadlock I think?
> 
> Martin Klapetek wrote:
>     Are we? I'm not sure really...how else could we do this then?
> 
> Martin Klapetek wrote:
>     Possibly we could cut off the $PID part of the service name and then simply check for that service, though this API seems more robust
> 
> Martin Gräßlin wrote:
>     I cannot mentally construct a dead lock situation. Could you please elaborate on what would dead lock?
> 
> David Edmundson wrote:
>     we're the kded, it's starting up.
>     We load the keyboard layout switching daemon
>     That tries creates a Status Notifier Item
>     
>     lib knotification queries if we're using the legacy tray, it's using the QPA so it's using this code all in the same process
>     That makes a DBus call to org.kde.StatusNotifierWatcher and blocks for a reply
>     
>     StatusNotifierWatcher is another kded module, it can't respond because we're blocked awaiting a reply from ourselves.

> lib knotification queries if we're using the legacy tray, it's using the QPA so it's using this code all in the same process

is that really the case? This code here should only be run if sni is not available.


- Martin


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


On Jan. 6, 2015, 7:16 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121885/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 7:16 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 339707
>     https://bugs.kde.org/show_bug.cgi?id=339707
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> -------
> 
> The "org.kde.StatusNotifierWatcher" is just a watcher/helper, not the actual systray object, that's "org.kde.StatusNotifierHost-$PID". Because Plasma appends the PID, we cannot check directly for it being present on the bus, so we check the org.kde.StatusNotifierWatcher.IsStatusNotifierHostRegistered property that's meant to be used for this.
> 
> Plus QSystemTrayIcon::isSystemTrayAvailable() is actually returning always true, because the Watcher is in kded and is /always/ present.
> 
> This also fixes many apps with KSNI crashing on plasma exit, bug 339707 (though I believe this is not the direct cause for that bug)
> 
> 
> Diffs
> -----
> 
>   src/platformtheme/kdeplatformsystemtrayicon.cpp b5e207c 
> 
> Diff: https://git.reviewboard.kde.org/r/121885/diff/
> 
> 
> Testing
> -------
> 
> Things do not crash anymore and the ::isSystemTrayAvailable() now returns correct value.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150107/92432452/attachment-0001.html>