-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix duplicated and crashing DBus tray icon #21980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
aec2e66
fdf755a
db0484e
5cd6efe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,10 @@ internal class DBusTrayIconImpl : ITrayIconImpl | |
| private (int, int, byte[]) _icon; | ||
|
|
||
| private string? _sysTrayServiceName; | ||
| // This task shows if the name request is in progress, complete, or failed. | ||
| private Task? _sysTrayServiceNameRequest; | ||
| // The bus keeps the name until the release is complete. A request before that fails. | ||
| private Task? _sysTrayServiceNameRelease; | ||
| private string? _tooltipText; | ||
| private bool _isDisposed; | ||
| private bool _serviceConnected; | ||
|
|
@@ -52,8 +56,8 @@ public DBusTrayIconImpl() | |
|
|
||
| MenuExporter = DBusMenuExporter.TryCreateDetachedNativeMenu(dbusMenuPath, _connection); | ||
|
|
||
| // CreateTrayIcon exports this object when the connection owns the name. | ||
| _statusNotifierItemDbusObj = new StatusNotifierItemDbusObj(_connection, dbusMenuPath); | ||
| _connection.AddMethodHandler(_statusNotifierItemDbusObj); | ||
| _statusNotifierItemDbusObj.ActivationDelegate += () => OnClicked?.Invoke(); | ||
|
|
||
| WatchAsync(); | ||
|
|
@@ -93,6 +97,10 @@ private async void WatchAsync() | |
| OnOwnerChanged(owner); | ||
| } | ||
| } | ||
| catch (OperationCanceledException) | ||
| { | ||
| // Dispose cancels this task. The continuation runs after Dispose sets _isDisposed. | ||
| } | ||
| catch (Exception e) when (!_isDisposed) | ||
| { | ||
| Logger.TryGet(LogEventLevel.Error, "DBUS") | ||
|
|
@@ -110,8 +118,6 @@ private void OnOwnerChanged(string? newOwner) | |
| _serviceConnected = true; | ||
| _statusNotifierWatcher = new StatusNotifierWatcher(_connection, "org.kde.StatusNotifierWatcher", "/StatusNotifierWatcher"); | ||
|
|
||
| DestroyTrayIcon(); | ||
|
|
||
| if (_isVisible) | ||
| CreateTrayIcon(); | ||
| } | ||
|
|
@@ -124,40 +130,112 @@ 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; | ||
|
|
||
| Task? request = null; | ||
|
|
||
| try | ||
| { | ||
| // Keep the name for the life of the icon. A new id shows a new item to the host. | ||
| 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}"); | ||
| } | ||
|
|
||
| if (_sysTrayServiceNameRelease is { } release) | ||
| { | ||
| await release; | ||
| if (ReferenceEquals(_sysTrayServiceNameRelease, release)) | ||
| _sysTrayServiceNameRelease = null; | ||
| if (!ShouldShowTrayIcon) | ||
| return; | ||
| } | ||
|
|
||
| _connection.RemoveMethodHandler(_statusNotifierItemDbusObj!.Path); | ||
| _connection.AddMethodHandler(_statusNotifierItemDbusObj); | ||
| request = _sysTrayServiceNameRequest ??= _connection.RequestNameAsync(_sysTrayServiceName); | ||
| await request; | ||
|
|
||
| _sysTrayServiceName = FormattableString.Invariant($"org.kde.StatusNotifierItem-{pid}-{tid}"); | ||
| await _connection.RequestNameAsync(_sysTrayServiceName); | ||
| await _statusNotifierWatcher.RegisterStatusNotifierItemAsync(_sysTrayServiceName); | ||
| // The icon can become hidden or disposed while the bus answers. The release then runs | ||
| // after this line. | ||
| if (!ShouldShowTrayIcon || !ReferenceEquals(_sysTrayServiceNameRequest, request)) | ||
| return; | ||
|
|
||
| _statusNotifierItemDbusObj!.SetTitleAndTooltip(_tooltipText); | ||
| _statusNotifierItemDbusObj.SetIcon(_icon); | ||
| // Export the object only while the connection owns the name. If not, a host that scans | ||
| // the bus adds a second item. | ||
| _connection.AddMethodHandler(_statusNotifierItemDbusObj); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If It is just an error in the log, but it points to We could improve with a small guard so re entry skips the add instead of throwing. |
||
|
|
||
| await _statusNotifierWatcher.RegisterStatusNotifierItemAsync(_sysTrayServiceName); | ||
|
|
||
| if (!ShouldShowTrayIcon) | ||
| return; | ||
|
|
||
| _statusNotifierItemDbusObj.SetTitleAndTooltip(_tooltipText); | ||
| _statusNotifierItemDbusObj.SetIcon(_icon); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| // Clear only this request, and only if it did not complete. The next call then asks for | ||
| // the name again. | ||
| if (request is { Status: not TaskStatus.RanToCompletion } && ReferenceEquals(_sysTrayServiceNameRequest, request)) | ||
| _sysTrayServiceNameRequest = null; | ||
|
|
||
| if (!_isDisposed) | ||
| Logger.TryGet(LogEventLevel.Error, "DBUS") | ||
| ?.Log(this, "Unable to register the system tray icon.\n{Exception}", e); | ||
| } | ||
| } | ||
|
|
||
| // CreateTrayIcon reads these conditions again after each await. | ||
| private bool ShouldShowTrayIcon => !_isDisposed && _isVisible && _serviceConnected; | ||
|
|
||
| private void DestroyTrayIcon() | ||
| { | ||
| if (_connection is null || !_serviceConnected || _isDisposed || _statusNotifierItemDbusObj is null || _sysTrayServiceName is null) | ||
| return; | ||
|
|
||
| _connection!.ReleaseNameAsync(_sysTrayServiceName); | ||
| _connection.RemoveMethodHandler(_statusNotifierItemDbusObj.Path); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Releases the bus name. A host removes the item when the name goes away. | ||
| /// Call this after <see cref="DestroyTrayIcon"/>, but not when the watcher stops. | ||
| /// </summary> | ||
| private void ReleaseTrayServiceName() | ||
| { | ||
| if (_connection is null || _sysTrayServiceName is null || _sysTrayServiceNameRequest is not { } request) | ||
| return; | ||
|
|
||
| _sysTrayServiceNameRequest = null; | ||
| _sysTrayServiceNameRelease = ReleaseTrayServiceName(request, _connection, _sysTrayServiceName); | ||
| } | ||
|
|
||
| private async Task ReleaseTrayServiceName(Task request, DBusConnection connection, string name) | ||
| { | ||
| try | ||
| { | ||
| // Wait for the request. A release before the connection owns the name has no effect. | ||
| await request; | ||
| await connection.ReleaseNameAsync(name); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| if (!_isDisposed) | ||
| Logger.TryGet(LogEventLevel.Error, "DBUS") | ||
| ?.Log(this, "Unable to release the system tray icon name.\n{Exception}", e); | ||
| } | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| IsActive = false; | ||
| DestroyTrayIcon(); | ||
| ReleaseTrayServiceName(); | ||
| (MenuExporter as IDisposable)?.Dispose(); | ||
| _watchCts?.Cancel(); | ||
| _isDisposed = true; | ||
|
|
@@ -215,6 +293,7 @@ public void SetIsVisible(bool visible) | |
| break; | ||
| case false when _isVisible: | ||
| DestroyTrayIcon(); | ||
| ReleaseTrayServiceName(); | ||
| break; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed this is the root cause of the duplicate. On
main, the object can be inspected through the connection immediately after construction, even before the name is owned. On this branch, it isn’t exposed until the name is acquired. Good catch.