| Summary: | "Add widgets..." and "Show alternatives..." options from context menu stop working after manually restarting plasmashell process using `plasmashell --replace` | ||
|---|---|---|---|
| Product: | [Plasma] plasmashell | Reporter: | Patrick Silva <bugseforuns> |
| Component: | general | Assignee: | David Edmundson <kde> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aspotashev, bizyaev, deresiant, doncbugs, firlaevhans.fiete, iodreamify, me, meven29, nate, openmindead, plasma-bugs, postix, qydwhotmail, waqar.17a |
| Priority: | NOR | ||
| Version: | 5.23.0 | ||
| Target Milestone: | 1.0 | ||
| Platform: | Arch Linux | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/plasma/plasma-workspace/commit/8056e09e08b37e19b1f55179fc8bfa960eea8e24 | Version Fixed In: | 5.26.4 |
| Sentry Crash Report: | |||
|
Description
Patrick Silva
2021-06-25 16:14:26 UTC
Can reproduce. Same here on Plasma 5.21.5, KF 5.83.0, Qt 5.15.2, X11. Basically the first time Plasma crashes or is manually restarted during development activities, these functions break. I've done some debugging and it appears that the problem is ShellCorona::handleContainmentAdded() not being run after plasmashell is restarted. This function is solely responsible for handling the signals that open those popups and handle the initial plasmoid setup script (if present), so when it doesn't get run, the connections don't get made and that functionality breaks. It appears that configDialog.containmentPlugin isn't set by the time the panel configuration window appears... Getting towards the end of my ability to debug this, I fear. Actually the same bug happens when you try to trigger the Widget Explorer from the context menu, so I guess it isn't getting set in multiple places, or else maybe the need for it to be set is unnecessary? Urgh the complexity of this code makes me want to cry Breakthrough thanks to rubber ducking with my son: restarting with plasmashell --replace causes this bug, but manually killing plasmashell and launching it manually with either `plasmashell` or `kstart5 plasmashell` avoids the bug. So there is something about plasmashell's --replace argument that triggers it... It appears that this connection in shellcorona.cpp: connect(this, &ShellCorona::containmentAdded, this, &ShellCorona::handleContainmentAdded); happens too late when using plasmashell --restart. In this case, the containmentAdded() signal in corona.cpp is emitted, but it doesn't do anything because the connection hasn't been made yet. When launching plasmashell but it's not already running, the signal gets emitted after the connection has been made, so it always works. The only difference seems to be this line in shell/main.cpp: KDBusService service(KDBusService::Unique | KDBusService::StartupOption(replace ? KDBusService::Replace : 0)); Hmm maybe the problem is in KDbusAddons? This doesn't look right...
if (options & KDBusService::Replace) {
//stuff
} else if (options & KDBusService::Unique) {
If I'm reading this right, when both KDBusService::Replace and KDBusService::Unique are specified, it seems like the code to handle KDBusService::Unique gets ignored.
Hmm, changing that doesn't seem to fix the problem... Giving up for now since this only affects developer/power user setups and has a trivial workaround (`killall plasmashell && kstart5 plasmashell`) You found all the parts.
Cold case:
- we call ShellCorona::load from startup
- we have no kactivities loaded yet, we do an early return and connect
- we call ShellCorona::init
- activities load, we start the real load via the connect
Restart case:
- we call ShellCorona::load
- we have no kactivities loaded yet, we do an early return and connect
- We start an event loop, waiting for our bus name to appear
- activities load, we start the real load
- we call ShellCorona::init
>Urgh the complexity of this code makes me want to cry
Was also a valid analysis.
*** Bug 443647 has been marked as a duplicate of this bug. *** *** Bug 445438 has been marked as a duplicate of this bug. *** *** Bug 445796 has been marked as a duplicate of this bug. *** I ran into this problem today but with the difference that "Show Alternatives.." is working for me but "Add Widgets" doesn't. Also, I am running plasma like normal, didn't restart it manually or anything so I think this is a non poweruser / developer bug. Plasma version 5.23.4 Frameworks: 5.89 Raising priority as it seems to be affecting users after Plasma crashes, not just developers. Not a regression in 5.25. *** Bug 456556 has been marked as a duplicate of this bug. *** A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/2127 Git commit d79a927a4c7a8b489b985a6ae94bd9d8f082db93 by David Edmundson. Committed on 20/09/2022 at 20:43. Pushed by davidedmundson into branch 'master'. [shell] Always call load after init Currently we get two different startup paths: Cold case: - we call ShellCorona::load from startup - we have no kactivities loaded yet, we do an early return and connect - we call ShellCorona::init - activities load, we start the real load via the connect Restart case: - we call ShellCorona::load - we have no kactivities loaded yet, we do an early return and connect - We start an event loop, waiting for our bus name to appear - activities load, we start the real load - we call ShellCorona::init Calling load before init results in broken behaviour as things aren't initialized. setShell is only called once in main.cpp really early on. It's not used elsewhere or exposed on DBus. There's no need for it to call load, when it will fail and wait for an event loop in almost all cases. Calling it from init makes ordering easier to follow and consistent through all paths. M +43 -28 shell/shellcorona.cpp https://invent.kde.org/plasma/plasma-workspace/commit/d79a927a4c7a8b489b985a6ae94bd9d8f082db93 Git commit d2b4d751e7723f40c14b0dea676ab5426d77340d by Fushan Wen. Committed on 21/09/2022 at 00:05. Pushed by fusionfuture into branch 'master'. shell: revert unrelated changes in d79a927a4c7a8b489b985a6ae94bd9d8f082db93 M +1 -21 shell/shellcorona.cpp https://invent.kde.org/plasma/plasma-workspace/commit/d2b4d751e7723f40c14b0dea676ab5426d77340d Git commit 8056e09e08b37e19b1f55179fc8bfa960eea8e24 by Devin Lin, on behalf of David Edmundson. Committed on 12/11/2022 at 15:23. Pushed by devinlin into branch 'Plasma/5.26'. [shell] Always call load after init Currently we get two different startup paths: Cold case: - we call ShellCorona::load from startup - we have no kactivities loaded yet, we do an early return and connect - we call ShellCorona::init - activities load, we start the real load via the connect Restart case: - we call ShellCorona::load - we have no kactivities loaded yet, we do an early return and connect - We start an event loop, waiting for our bus name to appear - activities load, we start the real load - we call ShellCorona::init Calling load before init results in broken behaviour as things aren't initialized. setShell is only called once in main.cpp really early on. It's not used elsewhere or exposed on DBus. There's no need for it to call load, when it will fail and wait for an event loop in almost all cases. Calling it from init makes ordering easier to follow and consistent through all paths. M +43 -28 shell/shellcorona.cpp https://invent.kde.org/plasma/plasma-workspace/commit/8056e09e08b37e19b1f55179fc8bfa960eea8e24 Git commit a77f1bda64114712603a8c25bc2c678dc6957676 by Devin Lin, on behalf of Fushan Wen. Committed on 12/11/2022 at 15:23. Pushed by devinlin into branch 'Plasma/5.26'. shell: revert unrelated changes in d79a927a4c7a8b489b985a6ae94bd9d8f082db93 M +1 -21 shell/shellcorona.cpp https://invent.kde.org/plasma/plasma-workspace/commit/a77f1bda64114712603a8c25bc2c678dc6957676 |