Fix duplicated and crashing DBus tray icon

Two problems in DBusTrayIconImpl, both visible on GNOME with the
appindicator/KStatusNotifierItem extension.

Duplicated icon: the StatusNotifierItem object was exported on the
connection before the well-known name was requested, and the name was
released and re-requested (with a new instance id) every time the
StatusNotifierWatcher came back. A host that scans the bus for items -
gnome-shell-extension-appindicator does this whenever it starts - finds
the object on a connection which does not own an
org.kde.StatusNotifierItem-* name, so it registers the item under the
unique connection name, and the RegisterStatusNotifierItem call that
follows adds a second entry for the same process. The object is now
exported only while the name is owned, the name keeps its instance id
for the lifetime of the tray icon, and it is no longer dropped and
retaken when the watcher restarts.

Hiding still releases the name: that is the only part of it a host can
observe, since the item stays Active and unexporting the object is
silent on the bus. It now happens after the object has been unexported,
so the "object exported while the name is not owned" window which
produces the duplicate never opens.

Crash on disposal: WatchAsync is an async void method awaiting a
cancellable delay. Dispose cancels the token and only then sets
_isDisposed, but the continuation is posted to the dispatcher and runs
after Dispose has returned, so neither exception filter matches and the
OperationCanceledException is rethrown on the dispatcher, terminating
the process. The cancellation is now handled explicitly. CreateTrayIcon
is async void as well and takes the process down the same way when a
DBus call fails (for example when the watcher disappears while the item
is being registered), so its body is guarded too.

Author: bolikcraft

src/Avalonia.FreeDesktop/DBusTrayIconImpl.cs

@@ -23,6 +23,7 @@ internal class DBusTrayIconImpl : ITrayIconImpl
         private (int, int, byte[]) _icon;
 
         private string? _sysTrayServiceName;
+        private bool _sysTrayServiceNameOwned;
         private string? _tooltipText;
         private bool _isDisposed;
         private bool _serviceConnected;
@@ -52,8 +53,8 @@ public DBusTrayIconImpl()
 
             MenuExporter = DBusMenuExporter.TryCreateDetachedNativeMenu(dbusMenuPath, _connection);
 
+            // The object is only exported once the well-known name is owned, see CreateTrayIcon.
             _statusNotifierItemDbusObj = new StatusNotifierItemDbusObj(_connection, dbusMenuPath);
-            _connection.AddMethodHandler(_statusNotifierItemDbusObj);
             _statusNotifierItemDbusObj.ActivationDelegate += () => OnClicked?.Invoke();
 
             WatchAsync();
@@ -93,6 +94,11 @@ private async void WatchAsync()
                     OnOwnerChanged(owner);
                 }
             }
+            catch (OperationCanceledException)
+            {
+                // The watch was cancelled by Dispose. Note that this can't be filtered on _isDisposed:
+                // the continuation is posted to the dispatcher and runs after Dispose has completed.
+            }
             catch (Exception e) when (!_isDisposed)
             {
                 Logger.TryGet(LogEventLevel.Error, "DBUS")
@@ -110,8 +116,6 @@ private void OnOwnerChanged(string? newOwner)
                 _serviceConnected = true;
                 _statusNotifierWatcher = new StatusNotifierWatcher(_connection, "org.kde.StatusNotifierWatcher", "/StatusNotifierWatcher");
 
-                DestroyTrayIcon();
-
                 if (_isVisible)
                     CreateTrayIcon();
             }
@@ -124,40 +128,79 @@ private void OnOwnerChanged(string? newOwner)
 
         private async void CreateTrayIcon()
         {
-            if (_connection is null || !_serviceConnected || _isDisposed || _statusNotifierWatcher is null)
+            if (_connection is null || !_serviceConnected || _isDisposed || _statusNotifierItemDbusObj is null || _statusNotifierWatcher is null)
                 return;
 
+            try
+            {
+                // The name is generated once and reused: taking a new instance id on every registration
+                // makes the host see a brand new item instead of the one it already knows about.
+                if (_sysTrayServiceName is null)
+                {
 #if NET5_0_OR_GREATER
-            var pid = Environment.ProcessId;
+                    var pid = Environment.ProcessId;
 #else
-            var pid = Process.GetCurrentProcess().Id;
+                    var pid = Process.GetCurrentProcess().Id;
 #endif
-            var tid = s_trayIconInstanceId++;
+                    var tid = s_trayIconInstanceId++;
+                    _sysTrayServiceName = FormattableString.Invariant($"org.kde.StatusNotifierItem-{pid}-{tid}");
+                }
 
-            _connection.RemoveMethodHandler(_statusNotifierItemDbusObj!.Path);
-            _connection.AddMethodHandler(_statusNotifierItemDbusObj);
+                if (!_sysTrayServiceNameOwned)
+                {
+                    // The flag is set before the call so that a hide racing with this one queues its
+                    // ReleaseName after the request below instead of skipping it altogether.
+                    _sysTrayServiceNameOwned = true;
+                    await _connection.RequestNameAsync(_sysTrayServiceName);
+                }
 
-            _sysTrayServiceName = FormattableString.Invariant($"org.kde.StatusNotifierItem-{pid}-{tid}");
-            await _connection.RequestNameAsync(_sysTrayServiceName);
-            await _statusNotifierWatcher.RegisterStatusNotifierItemAsync(_sysTrayServiceName);
+                // Only export the object once the name is owned: a host which scans the bus for items would
+                // otherwise find it on the unique connection name and register a second, duplicate item.
+                _connection.AddMethodHandler(_statusNotifierItemDbusObj);
 
-            _statusNotifierItemDbusObj!.SetTitleAndTooltip(_tooltipText);
-            _statusNotifierItemDbusObj.SetIcon(_icon);
+                await _statusNotifierWatcher.RegisterStatusNotifierItemAsync(_sysTrayServiceName);
+
+                _statusNotifierItemDbusObj.SetTitleAndTooltip(_tooltipText);
+                _statusNotifierItemDbusObj.SetIcon(_icon);
+            }
+            catch (Exception e)
+            {
+                if (!_isDisposed)
+                    Logger.TryGet(LogEventLevel.Error, "DBUS")
+                        ?.Log(this, "Unable to register the system tray icon.\n{Exception}", e);
+            }
         }
 
         private void DestroyTrayIcon()
         {
             if (_connection is null || !_serviceConnected || _isDisposed || _statusNotifierItemDbusObj is null || _sysTrayServiceName is null)
                 return;
 
-            _connection!.ReleaseNameAsync(_sysTrayServiceName);
             _connection.RemoveMethodHandler(_statusNotifierItemDbusObj.Path);
         }
 
+        /// <summary>
+        /// Gives up the well-known name, which is what makes a host drop the item: unexporting the
+        /// object is not observable on the bus at all. Must be called after <see cref="DestroyTrayIcon"/>
+        /// so that the object is never exported while the name is unowned - that is the state in which a
+        /// host scanning the bus registers the item under the unique connection name and ends up with a
+        /// duplicate. The name is not released when the watcher goes away: keeping it means a restarting
+        /// host always sees the object and the name together.
+        /// </summary>
+        private void ReleaseTrayServiceName()
+        {
+            if (_connection is null || _sysTrayServiceName is null || !_sysTrayServiceNameOwned)
+                return;
+
+            _sysTrayServiceNameOwned = false;
+            _connection.ReleaseNameAsync(_sysTrayServiceName);
+        }
+
         public void Dispose()
         {
             IsActive = false;
             DestroyTrayIcon();
+            ReleaseTrayServiceName();
             (MenuExporter as IDisposable)?.Dispose();
             _watchCts?.Cancel();
             _isDisposed = true;
@@ -215,6 +258,7 @@ public void SetIsVisible(bool visible)
                     break;
                 case false when _isVisible:
                     DestroyTrayIcon();
+                    ReleaseTrayServiceName();
                     break;
             }