Bug 416652 - Empty items in system tray area
Summary: Empty items in system tray area
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: System Tray (show other bugs)
Version: 5.17.90
Platform: Arch Linux Linux
: NOR normal
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-01-23 15:40 UTC by hexchain
Modified: 2020-02-03 15:57 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.18.0


Attachments
Empty items in tray area (70.42 KB, image/png)
2020-01-23 15:40 UTC, hexchain
Details
Empty items in configuration (150.06 KB, image/png)
2020-01-23 15:41 UTC, hexchain
Details
Script to list all Status Notifier Icons (535 bytes, application/x-shellscript)
2020-01-24 09:34 UTC, Konrad Materka
Details
Script output (1.58 KB, text/plain)
2020-01-24 10:09 UTC, hexchain
Details
session bus monitor logs (41.76 KB, application/gzip)
2020-01-30 11:02 UTC, hexchain
Details
wireshark dbus-session capture (9.08 KB, application/gzip)
2020-01-30 11:15 UTC, hexchain
Details
session bus monitor logs (cleared) (4.73 KB, application/gzip)
2020-01-30 22:36 UTC, Konrad Materka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description hexchain 2020-01-23 15:40:56 UTC
Created attachment 125329 [details]
Empty items in tray area

SUMMARY
There are a lot of empty items in the expanded "Status and Notifications" area. I haven't figured out how exactly to reproduce, nor how to remove them. I have tried to kquitapp5 and kstart5 plasmashell but they are still there.

Screenshots attached.

EXPECTED RESULT
There should not be such entries.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 5.17.90
KDE Frameworks Version: 5.66.0
Qt Version: 5.14.0
Comment 1 hexchain 2020-01-23 15:41:19 UTC
Created attachment 125330 [details]
Empty items in configuration
Comment 2 Kai Uwe Broulik 2020-01-23 15:41:39 UTC
Yeah I have noticed the same but also failed to find a pattern.
Comment 3 Nate Graham 2020-01-23 20:33:19 UTC
If you click on them, are they real? Or does nothing happen?
Comment 4 hexchain 2020-01-23 20:35:44 UTC
(In reply to Nate Graham from comment #3)
> If you click on them, are they real? Or does nothing happen?

Nothing happens.
Comment 5 Konrad Materka 2020-01-24 09:34:12 UTC
Created attachment 125360 [details]
Script to list all Status Notifier Icons

Both "hidden items" and "Entries configuration" are using the same data source, but entirely different rendering and model. I guess that these empty items are in the data source. Maybe something is polluting DBus service ("/StatusNotifierWatcher").

Can you use attached script to list all registered icons directly from the DBus service?
Comment 6 hexchain 2020-01-24 10:09:01 UTC
Created attachment 125361 [details]
Script output

The output of the script is attached. The number of non-exist names is the same as the number of empty items.
Comment 7 hexchain 2020-01-25 00:13:26 UTC
Somehow those empty icons disappeared, probably because I logged out once?
Comment 8 Konrad Materka 2020-01-25 11:36:18 UTC
There are some "ghost" items in StatusNotifierWatcher. After restart/log out they should disappear.
It is hard to tell what is causing this, probably some app is registering notification icons and removal is not notified correctly by the app. Or there is a bug in StatusNotifierWatcher and some items are not removed even if StatusNotifierWatcher is notified to remove them. Really hard to tell...
Comment 9 hexchain 2020-01-26 17:36:11 UTC
I have not seen any new empty items for several days. Looking at the log I suspect the empty items belong to Telegram prior to 1.9.3-1 (Arch package), because all dbus IDs of the stale items are registered before upgrading from that version, and after starting Telegram (supposedly) each time.

However, there are big changes between 1.9.3-1 and 1.9.4-2 so I couldn't really tell what has fixed the problem.
Comment 10 Nate Graham 2020-01-28 05:42:33 UTC
So this fixed itself? Is there anything we could conceivably fix here, Konrad?
Comment 11 Konrad Materka 2020-01-28 10:10:02 UTC
I'm not so familiar with DBus and StatusNotifierWatcher, but it looks that everything is OK on KDE side. It uses QDBusServiceWatcher to watch for service unregistration. There is a small possibility that QDBusServiceWatcher has a bug (unlikely) but most probably it was a bug in Telegram. I quickly checked Telegrams commit log, there was some work related to DBus, but for notifications, including https://blog.broulik.de/2020/01/venturing-out/ :)

If everything is ok now, I would close it.
Comment 12 hexchain 2020-01-28 10:22:23 UTC
(In reply to Konrad Materka from comment #11)
> I'm not so familiar with DBus and StatusNotifierWatcher, but it looks that
> everything is OK on KDE side. It uses QDBusServiceWatcher to watch for
> service unregistration. There is a small possibility that
> QDBusServiceWatcher has a bug (unlikely) but most probably it was a bug in
> Telegram. I quickly checked Telegrams commit log, there was some work
> related to DBus, but for notifications, including
> https://blog.broulik.de/2020/01/venturing-out/ :)
>

If you would like to, you could probably also check out https://git.archlinux.org/svntogit/community.git/log/trunk?h=packages/telegram-desktop. 

Telegram desktop client used to require lots of patches to work with system libraries. After 1.9.4, there are some build options to make it link to the system libraries so the patches are no longer required. As a result, some large chunks of code may get disabled/enabled, which may affect its SNI behavior.
 
> If everything is ok now, I would close it.

I'm not against it. I'll report back if anything new happens.
Comment 13 Konrad Materka 2020-01-28 10:40:36 UTC
(In reply to hexchain from comment #12)
> Telegram desktop client used to require lots of patches to work with system
> libraries. After 1.9.4, there are some build options to make it link to the
> system libraries so the patches are no longer required. As a result, some
> large chunks of code may get disabled/enabled, which may affect its SNI
> behavior.
Yes, this might be related, definitely.

I'm closing this. Feel free to reopen if it happens again.
Comment 14 hexchain 2020-01-30 11:02:15 UTC
Created attachment 125539 [details]
session bus monitor logs

I have encountered this problem again today, with telegram-desktop 1.9.9-1 on Arch Linux.

Some system logs after starting Telegram:
> kded5[1952]: Registering ":1.765/StatusNotifierItem" to system tray
> kded5[1952]: Registering ":1.766/StatusNotifierItem" to system tray

And after quitting Telegram (tray icon right click -> Quit):
> kded5[1952]: Service  ":1.766" unregistered

I have managed to reproduce the problem with `busctl --user monitor` running. The log file is attached.
Comment 15 hexchain 2020-01-30 11:15:42 UTC
Created attachment 125540 [details]
wireshark dbus-session capture

This is another try. Relevant syslog:

After starting Telegram:
> kded5[1952]: Registering ":1.840/StatusNotifierItem" to system tray
> kded5[1952]: Registering ":1.841/StatusNotifierItem" to system tray
> kded5[1952]: Service  ":1.841" unregistered
> kded5[1952]: Registering ":1.842/StatusNotifierItem" to system tray
And after quitting:
> kded5[1952]: Service  ":1.842" unregistered
This time I have wireshark listening on dbus-session. The attached capture file is filtered with:
> dbus.value.str contains ":1.84"
Comment 16 Konrad Materka 2020-01-30 22:36:27 UTC
Created attachment 125558 [details]
session bus monitor logs (cleared)

My investigation:

There are 3 parties involved:
:1.10 = org.kde.StatusNotifierWatcher
:1.33 = org.freedesktop.Notifications = com.canonical.Unity = plasmashell
:1.763 - Telegram

Tray icon is created twice:
> Sender=:1.33  Destination=org.kde.StatusNotifierWatcher  Path=/StatusNotifierWatcher  Interface=org.kde.StatusNotifierWatcher  Member=RegisterStatusNotifierItem -> ":1.765"
> Sender=:1.763  Destination=org.kde.StatusNotifierWatcher  Path=/StatusNotifierWatcher  Interface=org.kde.StatusNotifierWatcher  Member=RegisterStatusNotifierItem -> ":1.766"

The ":1.765" is immediately destroyed. I was able to observe similar behavior with snap package on my computer to.

There is a race condition, if service is destroyed too soon, StatusNotifierWatcher can't notice it. From StatusNotifierWatcher cpp:

//check if the service has registered a SystemTray object
org::kde::StatusNotifierItem trayclient(service, path, QDBusConnection::sessionBus());
if (trayclient.isValid()) { // it exists here
    m_registeredServices.append(notifierItemId); // it dies here
    m_serviceWatcher->addWatchedService(service); // it is dead by now, no chance to get notification
    emit StatusNotifierItemRegistered(notifierItemId);
}

Race condition is a problem on it's own, but bigger issue is: why plasmashell is creating SNI? For a brief moment there a SNI object with hidden menu with only one item: toggled checkbox.
Comment 17 Konrad Materka 2020-01-31 14:36:58 UTC
Found it!

On start Telegram opens microphone, but only for a very short moment. This is detected by microphone indicator and notify icon is automatically created. Immediately after that indicator icon is destroyed, before StatusNotifierWatcher is able to detect that (race condition).
Comment 18 Konrad Materka 2020-02-03 13:12:36 UTC
Fix:
https://phabricator.kde.org/D27126
Comment 19 Konrad Materka 2020-02-03 15:57:42 UTC
Git commit 31303ad3542b6d05537b733e08857c43622ecc93 by Konrad Materka.
Committed on 03/02/2020 at 15:57.
Pushed by kmaterka into branch 'Plasma/5.18'.

[SNI] Fix race condition in item registration

Summary:
If StatusNotifierItem is registered and then immediately destroyed, it is possible that QDBusServiceWatcher will not emit the serviceUnregistered signal.
Add an additional check to avoid such situations.
FIXED-IN: 5.18.0

Test Plan: Telegram should not add empty items.

Reviewers: #plasma_workspaces, #plasma, davidedmundson

Reviewed By: #plasma_workspaces, #plasma, davidedmundson

Subscribers: davidedmundson, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D27126

M  +11   -7    statusnotifierwatcher/statusnotifierwatcher.cpp

https://commits.kde.org/plasma-workspace/31303ad3542b6d05537b733e08857c43622ecc93