Skip to content

Linux desktop notifications no longer freeze the main process (salvage #109623) - #118197

Merged
kshitijk4poor merged 6 commits into
NousResearch:mainfrom
kshitijk4poor:fix/desktop-linux-notification-freeze-109623
Sep 21, 2026
Merged

kshitijk4poor merged 6 commits into
NousResearch:mainfrom
kshitijk4poor:fix/desktop-linux-notification-freeze-109623

Conversation

@kshitijk4poor

Copy link
Copy Markdown

Linux desktop notifications no longer freeze the main process when the notification daemon stalls; they go over D-Bus asynchronously and fail with a bounded false instead.

Salvage of #109623 by @BearHuddleston (both commits cherry-picked with authorship preserved; clean apply on current main) plus four follow-ups from review.

Why

Electron's Notification on Linux calls libnotify synchronously. A daemon that is slow to activate, hung, or restarting blocks the main thread for the duration (observed while working on Bot Screen, #108914). Windows and macOS are unaffected and keep the Electron path.

Changes

Contributor commits (@BearHuddleston):

  • apps/desktop/electron/notification-linux.ts (new): freedesktop org.freedesktop.Notifications transport over the session bus using the pure-JS dbus-native@0.15.2. Own Hello, GetNameOwner → StartServiceByName fallback → GetCapabilities → Notify with the same payload shape libnotify sends (default/View + indexed actions, desktop-entry, suppress-sound, no expiry). 5 s per-call timeout with AbortController, 10 s cooldown after a failure, NameOwnerChanged fencing so a restarted daemon's reused IDs never fire the old notification's callbacks, sender fencing on ActionInvoked/NotificationClosed.
  • notification-ipc.ts: picks the Linux transport on linux, otherwise unchanged Electron Notification; peer renderers now share the in-flight delivery promise for the same dedupe key.
  • notification-registry.ts: releaseOnClose option (Linux only) so a daemon-dismissed notification is released instead of held for the 10 min TTL.

Follow-ups (ours):

  • refactor: drop fences that cannot execute (finished after an aborted invoke, connection !== state alongside the generation check, if (finished) inside a deleted map entry); a malformed Notify id gets its own error.
  • fix: a benign daemon swap mid-call no longer arms the 10 s global cooldown; Linux has no fallback, so that window dropped every notification exactly when the new daemon was healthy. Failures are classified by generation (owner still current → daemon's fault → cooldown; owner replaced since → none), because on a real bus a call to the vanished unique name comes back as NameHasNoOwner, not as the transport's own fence error. The fixture now answers stale unique names that way, and the race test asserts the next notification reaches the new owner immediately.
  • test: registerNativeNotifications takes an injectable platform. The only vitest lane runs on ubuntu, and the ipc test had mocked the Linux module, so the Electron branch that Windows/macOS run had zero CI coverage; the Linux tests lose their skipIf. 8 tests now run on every host (was 5 + 3 skipped on macOS).
  • fix: dispose() closes the bus socket from the existing will-quit teardown in main.ts, matching the file's keep-alive socket policy.

Dependency: dbus-native@0.15.2 (one runtime dep, xml2js; no native code, no dynamic requires). Lockfile delta is that closure plus sax flipping dev→prod. Bundled into dist/electron-main.mjs by the existing esbuild script; nothing needs to be external.

Verification

Check Result
tsc -p tsconfig.electron.json, eslint on touched files clean
vitest notification-{linux,ipc,registry}.test.ts on macOS host 8/8 (contributor head: 5 pass, 3 skipped)
Mutation: Linux transport not selected / releaseOnClose off / delivery timeout removed / cooldown exemption removed / dispose no-op / Notification.isSupported false each turns the binding test red
Real-wire Linux E2E on the final branch (Docker ubuntu:24.04: dbus-daemon --session + stub org.freedesktop.Notifications, esbuild-bundled transport) healthy delivery ~7 ms with the libnotify payload shape; click and action round-trip; stalled Notify/GetCapabilities → false at 5.0 s with the event loop alive (53 heartbeats during the stall) and the next call cooled down; absent service and dead bus → false in <10 ms; daemon killed and replaced while GetCapabilities is in flight → false at 0.9 s, next notification delivered 12 ms later (no cooldown)
main-boot-smoke.sh (main.ts touched) boots
esbuild bundle dbus-native inlined; only Node builtins remain required

Non-Linux behaviour traced unchanged: linux is undefined, so the isSupported gate, dedupe key, new Notification(options), registry retain and show() order are as before; the renderer discards the notify() result (void window.hermesDesktop?.notify(...)).

Fork CI never ran on #109623 (action_required), so the above is the first execution evidence for this change.

Closes #109623

BearHuddleston and others added 6 commits September 21, 2026 18:32
…transport

`finished` cannot be observed inside show(): close() aborts the signal
first, and dbus-native settles an aborted invoke synchronously, so the
awaiting code throws instead of resuming past the check. `connection !==
state` is implied by the generation check (disconnect bumps the
generation before clearing the connection), `connection !== target.state`
in close() is implied by `delivered` (disconnect fails every live entry,
which clears it), and receive() can never run after release() deleted its
map entry.

A malformed Notify id now gets its own error instead of being reported as
an owner change.
…Linux

Every failure in show() armed the global 10 s cooldown, including the
ones caused by the notification daemon being replaced mid-call: the
owner-changed fences, and the bus's NameHasNoOwner error for a call
already addressed to the vanished unique name. Linux has no Electron
fallback, so every notification in that window was silently dropped
exactly when the new daemon was healthy.

Classify by generation rather than by error: a failure against an owner
that is still current is the daemon's fault and cools down; one against
an owner that has since been replaced does not. The fixture now answers
calls to a stale unique name with NameHasNoOwner like the real bus, and
the race test asserts the next notification reaches the new owner
immediately.
The transport was picked from process.platform, so the ipc test mocked
the Linux module and the only vitest lane (ubuntu) never exercised the
Electron Notification branch that Windows and macOS run, while the Linux
tests skipped everywhere else. Make the platform an injectable host
parameter: the ipc test drives the Electron branch as darwin, the Linux
tests pass linux explicitly and lose their skipIf.
The session-bus socket was created lazily and never closed. main.ts
already tears down its pooled keep-alive sockets in will-quit so nothing
holds the event loop open or leaks an fd past app teardown; the new
long-lived socket joins that policy. Disposal goes through the existing
close path, so live notifications are failed the same way a daemon crash
fails them.
@kshitijk4poor
kshitijk4poor requested a review from a team September 21, 2026 13:13
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) labels Sep 21, 2026
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) September 21, 2026 13:20
@alt-glitch
alt-glitch self-requested a review September 21, 2026 13:56
@kshitijk4poor
kshitijk4poor merged commit 8f7b3b4 into NousResearch:main Sep 21, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants