1909879 - Nightly/Beta Flatpak crashes on startup if missing access to `org.kde.*` own-name

Status: RESOLVED FIXED

(Thunderbird :: OS Integration, defect, P1)

Description

[bbhtt]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:130.0) Gecko/20100101 Firefox/130.0

Steps to reproduce:

I repackage the official daily tarballs in Flatpak here https://gitlab.com/projects261/thunderbird-nightly-flatpak

Since tomorrow opening the Flatpak immediately crashes it. Unfortunately I could not get it to submit crash reports.

To reproduce:

• git clone https://gitlab.com/projects261/thunderbird-nightly-flatpak.git
• git checkout b51646e4fa444f1a4d9278a5747cb6e40107fcdf
• flatpak-builder --force-clean --sandbox --user --install --install-deps-from=flathub builddir org.mozilla.ThunderbirdNightly.yaml
• flatpak run org.mozilla.ThunderbirdNightly

Actual results:

I think after the tray work landed, the Flatpak started to crash because it has no access to --own-name=org.kde.*.

The tray seems to need access to org.kde.StatusNotifierItem-$ID-$PID but since ID and PID can change we usually have to give it access to whole of org.kde.*.

Expected results:

The lack of permission should not cause it to crash. Due to the crash there is no way to open Thunderbird without --own-name=org.kde.*

Comment 1

[bbhtt]

This will be relevant for the official Flatpak too once the tray work lands in a stable release. That one needs --own-name=org.kde.* access too for a working tray.

But the crash should be fixed first.

Comment 2

[bbhtt]

Another way to reproduce the crash is to install the latest build from the repo https://gitlab.com/projects261/thunderbird-nightly-flatpak#install and if Flatseal https://flathub.org/apps/com.github.tchx84.Flatseal is installed revoke access to org.kde.* for the app, then attempt to start it.

Comment 3

[bbhtt]

Managed to get a crash report https://crash-stats.mozilla.org/report/index/e5f5b695-d354-4271-b39a-caa280240725

Comment 4

[Wayne Mery (:wsmwk)]

Crash report: https://crash-stats.mozilla.org/report/index/e5f5b695-d354-4271-b39a-caa280240725

MOZ_CRASH Reason: called `Result::unwrap()` on an `Err` value: D-Bus error: org.freedesktop.DBus.Error.ServiceUnknown (org.freedesktop.DBus.Error.ServiceUnknown)

Top 10 frames:

0  libxul.so  MOZ_Crash(char const*, int, char const*)  mfbt/Assertions.h:317
0  libxul.so  RustMozCrash  mozglue/static/rust/wrappers.cpp:18
1  libxul.so  mozglue_static::panic_hook  mozglue/static/rust/lib.rs:102
1  libxul.so  core::ops::function::Fn::call  library/core/src/ops/function.rs:79
2  libxul.so  <alloc::boxed::Box<F, A> as core::ops::function::Fn<Args>>::call  library/alloc/src/boxed.rs:2036
2  libxul.so  std::panicking::rust_panic_with_hook  library/std/src/panicking.rs:799
3  libxul.so  std::panicking::begin_panic_handler::{{closure}}  library/std/src/panicking.rs:664
4  libxul.so  std::sys_common::backtrace::__rust_end_short_backtrace  library/std/src/sys_common/backtrace.rs:171
5  libxul.so  rust_begin_unwind  library/std/src/panicking.rs:652
6  libxul.so  core::panicking::panic_fmt  library/core/src/panicking.rs:72

Comment 5

[Wayne Mery (:wsmwk)]

magnus, any ideas?

Comment 6

[Gummi]

Looking at the backtrace, and in particular ksni/src/service.rs, it seems like Chromium has solved a similar issue before:
https://github.com/iovxw/ksni/pull/25
https://chromium-review.googlesource.com/c/chromium/src/+/4179380

And the fix would be to use spawn_without_bus_name instead of spawn in rust/sys_tray/src/linux/mod.rs when running from flatpak.

I'm willing to send a patch, but how does Thunderbird detect flatpak?

Comment 7

[Gummi]

Actually it's possible to use spawn_without_bus_name unconditionally, quoting the Chromium issue:
https://issues.chromium.org/issues/40253493

The StatusIconLinuxDbus class tries to register org.kde.StatusNotifierItem-$PID-$ID on the session bus:

While this is correct to the StatusNotifier spec, its actually a very problematic thing to do. The relevant implementations of StatusNotifier either stopped doing it, such as Qt/KDE, or never did it such as libappindicator.

See also: https://github.com/KDE/knotifications/commit/2b0b904ee0e986ea0a96a8f54e1522e492885ffe

The reason this is problematic is applications are more regularly becoming sandboxed on Linux via Flatpak or Snap. These applications require explicit permission to own well-known names on the session bus.

There is no actual upside to requesting ownership as the StatusNotifierWatcher.RegisterStatusNotifierItem() API takes an arbitrary string for the service name, and you can give it your unique-name instead, this does and will work fine on all implementations as libappindciator has done this for as long as the library has existed and is an extremely broadly used implementation.

Comment 8

[Gummi]

Attachment: 0001-Bug-1909879-Fix-flatpak-crash-caused-by-failure-to-r.patch

Comment 9

[Gummi]

Attachment: 0001-Bug-1909879-Fix-flatpak-crash-caused-by-failure-to-r.patch

Fix attached, please review

Comment 10

[Magnus Melin [:mkmelin]]

Please submit the patch through phabricator. https://developer.thunderbird.net/thunderbird-development/fixing-a-bug#submitting-a-patch
You can put " r=ikey" in the commit message to send the review to him

Comment 11

[Gummi]

Attachment: Bug 1909879 - Fix flatpak crash caused by failure to register systray bus name, r=ikey

As a sandboxed app, it is better to implement StatusNotifierItem on the bus
name we already own rather than on a well-known bus name that will clash
(due to predictable PIDs inside sandboxes) or be denied.

The bus name being denied by default caused Thunderbird to crash
when run from flatpak.

More info here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1909879#c7
https://issues.chromium.org/issues/40253493

Comment 12

[Gummi]

Here it is on phabricator: https://phabricator.services.mozilla.com/D219037
r=ikey; if someone with privileges could trigger a build that would be great.

Comment 13

[Wayne Mery (:wsmwk)]

This is #1 crash, despite linux being our smallest user population.

Comment 14

[Gummi]

Should I pick another reviewer so this can get out to flatpak users?
I think ikey was involved in the initial implementation, but anyone with some dbus knowledge should be able to review this one-line change. The quote above from the chromium bug tracker explains it well, and I have also built flatpaks before/after to confirm the fix works as intended.

Comment 15

[Corey Bryant]

Ikey, would you be the right person to review Gummi's patch?

Comment 16

[Wayne Mery (:wsmwk)]

#1 beta crash, still

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9f7a5393399a
Fix flatpak crash caused by failure to register systray bus name, r=ikey

Comment 18

[Gummi]

The fix is not part of today's flatpak beta build (131b03, https://hg.mozilla.org/releases/comm-beta/ I think).
Can someone mark it for backporting?
Currently the flatpak beta channel is useless, as it has been crashing for more than a month.

Comment 19

[Alessandro Castellani [:aleca]]

Thanks for the ping, a need-info is enough for a reminder, no need to reopen the bug.
I'll request beta uplift.

Comment 20

[Alessandro Castellani [:aleca]]

Comment on attachment 9418793 [details]
Bug 1909879 - Fix flatpak crash caused by failure to register systray bus name, r=ikey

[Approval Request Comment]
Regression caused by (bug #): bug 18732
User impact if declined: flatpak bustage
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 21

[Corey Bryant]

Comment on attachment 9418793 [details]
Bug 1909879 - Fix flatpak crash caused by failure to register systray bus name, r=ikey

[Triage Comment]
Approved for beta

Comment 22

[Daniel Darnell [:dandarnell]]

Thunderbird 131.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/a927468bd2de

Comment 23

[Wayne Mery (:wsmwk)]

core::result::unwrap_failed | ksni::service::TrayService<T>::spawn_without_dbus_name::{{closure}} has the same start and end as this bugs initial stack signature

Crash has stopped according to crash-stats.

Comment 24

[Wayne Mery (:wsmwk)]

Crash has stopped according to crash-stats.

Especially for Freedesktop SDK 23.08 (Flatpak runtime)

However, beta crashes have only almost stopped. 132.0b6 had some [1].

And crashes on nightly have definitely not stopped for Archlinux on 132, 133, 134.

• https://crash-stats.mozilla.org/signature/?release_channel=nightly&platform_pretty_version=%3DArch%20Linux&product=Thunderbird&date=%3E%3D2024-10-02T13%3A10%3A00.000Z&date=%3C2024-11-02T13%3A10%3A00.000Z&_sort=user_comments&_sort=-date&signature=core%3A%3Aresult%3A%3Aunwrap_failed%20%7C%20ksni%3A%3Aservice%3A%3ATrayService%3CT%3E%3A%3Aspawn_without_dbus_name%3A%3A%7B%7Bclosure%7D%7D
• [1] https://crash-stats.mozilla.org/signature/?signature=core%3A%3Aresult%3A%3Aunwrap_failed%20%7C%20ksni%3A%3Aservice%3A%3ATrayService%3CT%3E%3A%3Aspawn_without_dbus_name%3A%3A%7B%7Bclosure%7D%7D