Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
50 stars 9 forks source link

Logged out from GNOME when changing settings #98

Closed DeN-AlB closed 3 months ago

DeN-AlB commented 3 months ago

Would be great to get support for Gnome 46. :)

Batwam commented 3 months ago

Thanks for reaching out, it's on the todo list. When I checked using Gnome OS Nightly a few weeks ago, everything seemed to work so the extension should work as-is. I downloaded Ubuntu 24.10 nightly build over the weekend to check but it still had gnome 45.3 ( I believe that there was a package dependency blocking the upgrade)...

We'll update the version as soon as we've had a chance to test it. In the meantime, you can enable the extension on Gnome 46 by disabling version check gsettings set org.gnome.shell disable-user-extensions true. To reset that, run gsettings set org.gnome.shell disable-user-extensions false.

Do let us know if you had a chance to use it under Gnome 46 as well ;-)

DeN-AlB commented 3 months ago

but it still had gnome 45.3

Arch Linux got an official release for Gnome 46 some days ago. That's why lots of users need new extension versions at the moment. :wink:

Do let us know if you had a chance to use it under Gnome 46 as well ;-)

Installed the extension, and it's looking good so far. Tried with Spotify and YouTube in Firefox. The only thing that happened was that I was automatically logged out of the Gnome session when switching the position from right to left. There was no message that I have to log out and log in again. The screen went black, and I had to log in again. Maybe this could be an issue?

Batwam commented 3 months ago

The screen went black, and I had to log in again.

Thanks for sharing, does this happen again if you switch right/left again? I am not aware of any changes in Gnome 46 which would cause this.

DeN-AlB commented 3 months ago

does this happen again if you switch right/left again?

No. Now I'm able to switch the position without logging out of the session now. But it happened also when changing the setting for "Extension index" for the first time. Now I'm able to change the index without logging out.

Anything interesting in the logs that might explain what happened at the time of the crash?

journalctl /usr/bin/gnome-session is empty. There are a lot of warnings and errors in journalctl /usr/bin/gnome-shell, but I don't know if they are because of that issue. :thinking:

Batwam commented 3 months ago

I just tested using Fedora rawhide and was able to change the settings without crashing gnome. Not sure what's causing your issue.

If you want to narrow down the journalctl output, you could limit to lines with moon-0xxf and exclude some of the normal ones: journalctl | grep -v [inflating\|creating\|extracting] | grep moon-0xff -A5 -B5. This said, I'm not convinced that the issue is directly caused by the extension if it can't be reproduced.

DeN-AlB commented 3 months ago

If you want to narrow down the journalctl output, you could limit to lines with moon-0xxf and exclude some of the normal ones:

Nothing found.

[denalb@PROMETHEUS-Archlinux ~]$ journalctl | grep -v [inflating\|creating\|extracting] | grep moon-0xff -A5 -B5
[denalb@PROMETHEUS-Archlinux ~]$
Batwam commented 3 months ago

sorry, I mistyped. Please try journalctl /usr/bin/gnome-shell | grep -v 'inflating\|creating\|extracting\|Activating' | grep moon-0xff -A5 -B5 instead.

DeN-AlB commented 3 months ago

Ok. Got some issues ...

[denalb@PROMETHEUS-Archlinux ~]$ journalctl  /usr/bin/gnome-shell | grep -v 'inflating\|creating\|extracting\|Activating' | grep moon-0xff -A5 -B5
                                                           @resource:///org/gnome/shell/ui/init.js:21:20

Mär 27 08:05:11 PROMETHEUS-Archlinux gnome-shell[1541]: Window manager warning: Ping serial 2893308 was reused for window W47, previous use was for window 0x2200003.
Mär 27 08:05:45 PROMETHEUS-Archlinux gnome-shell[1541]: Object St.Bin (0x5731ddcc58a0), has been already disposed — impossible to access it. This might be caused by the object having been destroyed from C code using something such as destroy(), dispose(), or remove() vfuncs.
                                                         == Stack trace for context 0x5731d917f620 ==
                                                         #0   5731d9246aa8 i   file:///home/denalb/.local/share/gnome-shell/extensions/mprisLabel@moon-0xff.github.com/extension.js:119 (204f11a69fb0 @ 90)
                                                         #1   5731d9246a18 i   resource:///org/gnome/shell/ui/init.js:21 (1c4af4270bf0 @ 48)
Mär 27 08:05:45 PROMETHEUS-Archlinux gnome-shell[1541]: Object St.Bin (0x5731ddcc58a0), has been already disposed — impossible to access it. This might be caused by the object having been destroyed from C code using something such as destroy(), dispose(), or remove() vfuncs.
                                                         == Stack trace for context 0x5731d917f620 ==
                                                         #0   5731d9246aa8 i   file:///home/denalb/.local/share/gnome-shell/extensions/mprisLabel@moon-0xff.github.com/extension.js:123 (204f11a69fb0 @ 207)
                                                         #1   5731d9246a18 i   resource:///org/gnome/shell/ui/init.js:21 (1c4af4270bf0 @ 48)
Mär 27 08:05:47 PROMETHEUS-Archlinux gnome-shell[9455]: Running GNOME Shell (using mutter 46.0) as a Wayland display server
Mär 27 08:05:47 PROMETHEUS-Archlinux gnome-shell[9455]: Made thread 'KMS thread' realtime scheduled
Mär 27 08:05:47 PROMETHEUS-Archlinux gnome-shell[9455]: Device '/dev/dri/card1' prefers shadow buffer
Mär 27 08:05:47 PROMETHEUS-Archlinux gnome-shell[9455]: Added device '/dev/dri/card1' (amdgpu) using atomic mode setting.
--

Mär 27 08:07:10 PROMETHEUS-Archlinux gnome-shell[9931]: Unhandled promise rejection. To suppress this warning, add an error handler to your promise chain with .catch() or a try-catch block around your await expression. Stack trace of the failed promise:
                                                         _promisify/proto[asyncFunc]@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:448:16
                                                         openExtensionPrefs@resource:///org/gnome/shell/ui/extensionSystem.js:320:26
                                                         openPreferences@resource:///org/gnome/shell/extensions/extension.js:27:26
                                                         _buildMenu/<@file:///home/denalb/.local/share/gnome-shell/extensions/mprisLabel@moon-0xff.github.com/extension.js:414:104
                                                         activate@resource:///org/gnome/shell/ui/popupMenu.js:195:14
                                                         _init/<@resource:///org/gnome/shell/ui/popupMenu.js:112:24
                                                         @resource:///org/gnome/shell/ui/init.js:21:20
Mär 27 08:10:20 PROMETHEUS-Archlinux gnome-shell[9931]: endSessionDialog: No XDG_SESSION_ID, fetched from logind: 7
Mär 27 08:10:22 PROMETHEUS-Archlinux gnome-shell[9931]: Shutting down GNOME Shell
--
                                                           _init/<@file:///home/denalb/.local/share/gnome-shell/extensions/appindicatorsupport@rgcjonas.gmail.com/dbusProxy.js:40:43
                                                           @resource:///org/gnome/shell/ui/init.js:21:20

Mär 27 08:28:23 PROMETHEUS-Archlinux gnome-shell[1471]: Object St.Bin (0x5b4bdc210ee0), has been already disposed — impossible to access it. This might be caused by the object having been destroyed from C code using something such as destroy(), dispose(), or remove() vfuncs.
                                                         == Stack trace for context 0x5b4bd8f316e0 ==
                                                         #0   5b4bd8ff8b28 i   file:///home/denalb/.local/share/gnome-shell/extensions/mprisLabel@moon-0xff.github.com/extension.js:119 (673b2cab1f0 @ 90)
                                                         #1   5b4bd8ff8a98 i   resource:///org/gnome/shell/ui/init.js:21 (56a16a70bf0 @ 48)
Mär 27 08:28:23 PROMETHEUS-Archlinux gnome-shell[1471]: Object St.Bin (0x5b4bdc210ee0), has been already disposed — impossible to access it. This might be caused by the object having been destroyed from C code using something such as destroy(), dispose(), or remove() vfuncs.
                                                         == Stack trace for context 0x5b4bd8f316e0 ==
                                                         #0   5b4bd8ff8b28 i   file:///home/denalb/.local/share/gnome-shell/extensions/mprisLabel@moon-0xff.github.com/extension.js:123 (673b2cab1f0 @ 207)
                                                         #1   5b4bd8ff8a98 i   resource:///org/gnome/shell/ui/init.js:21 (56a16a70bf0 @ 48)
Mär 27 08:28:26 PROMETHEUS-Archlinux gnome-shell[6338]: Running GNOME Shell (using mutter 46.0) as a Wayland display server
Mär 27 08:28:26 PROMETHEUS-Archlinux gnome-shell[6338]: Made thread 'KMS thread' realtime scheduled
Mär 27 08:28:26 PROMETHEUS-Archlinux gnome-shell[6338]: Device '/dev/dri/card1' prefers shadow buffer
Mär 27 08:28:26 PROMETHEUS-Archlinux gnome-shell[6338]: Added device '/dev/dri/card1' (amdgpu) using atomic mode setting.
[denalb@PROMETHEUS-Archlinux ~]$ 
Batwam commented 3 months ago

That's interesting as extension.js line 119 and 123 do indeed relate to relocating the extension left/right. Even more interesting is that I just tried to change the setting on my system (gnome 45.3) and gnome-shell froze as well!

Looks like you found something worth investigating 🧐

DeN-AlB commented 3 months ago

Looks like you found something worth investigating 🧐

Great! Hopefully you could find a solution. :+1:

Moon-0xff commented 3 months ago

I already encountered something similar:
When changing quickly between left/center/right sometimes the shell rebooted itself. A shell reboot only makes the screen go dark for a second, and the windows are rearranged a bit, no reason for a fix on my opinion.
A log-out is absolutely a reason to fix it.

My theory is/was that as there's a brief period when the widget doesn't exist, if a signal function (async) tries to interact with it, it errors out, however instead of erroring out gracefully, probably segfaults.
The shared logs might confirm my theory, or at least I'm not so far off.

It might be possible to fix this simply by checking if the widget exists before trying to interact with it, but the position change could happen between the check and the interaction so it would segfault anyway.

Note: a similar reset inducing bug I mentioned briefly

Moon-0xff commented 3 months ago

I made a commit in a new branch to address this. However, reading the thread again I realize that the issue is also caused by other settings, if this is true then we will need to change the way we apply a lot of settings.

If the above commit solves the issues with the panel position setting, then we can at least know that stopping the refresh function indeed prevents the shell from crashing.
Other async functions that could interfere couldn't be triggered at the same time as they require human input, so no problem there, the only function that needs to be stopped is _refresh.

Moon-0xff commented 3 months ago

Do note that I hijacked the title to reflect the already hijacked thread!

Regarding GNOME 46 we only needed to test it, assuming the extension worked fine on testing then it is/was just a matter of updating the metadata of the extension.

Of course this log-out issue takes priority, as the extension ~kicking~ logging you out from GNOME is a nasty bug!

DeN-AlB commented 3 months ago

Do note that I hijacked the title to reflect the already hijacked thread!

This is fine for me. :+1:

Batwam commented 3 months ago

@DeN-AlB are you able to test the new branch and see if you are still able to reproduce the crash? I'll give it a try when I get home.

Looking at the code again, moving the extension right/left and changing the index relies on the same part of the code which is referred to in the crash log so it would make sense for both issues to get fixed by this update.

DeN-AlB commented 3 months ago

@DeN-AlB are you able to test the new branch and see if you are still able to reproduce the crash?

Yes. I would remove the installed extension and could install the new one. But where can I get it from? I don't know how to install the extension without Extension Store.

Batwam commented 3 months ago

same instructions as here but for the logout-possible-fix branch rather than stable so the commands become

git clone https://github.com/Moon-0xff/gnome-mpris-label.git
cd gnome-mpris-label
git checkout logout-possible-fix
sh install.sh

You can leave the files there if it all works as any newer version will automatically overwrite these files. You can delete the gnome-mpris-label folder created by git once the installation is done as the script will copy the files so you won't need them anymore.

for info, I tried it and didn't get any crash.

DeN-AlB commented 3 months ago

Removed the old version and installed the fix. So version 28 is installed.

All settings were there already. But tried to switch the position from left to middle to right and back to left. No logout. Also changed the setting for the index, and thus I wasn't logged out from Gnome.

Maybe I had to delete the configuration? But where is it stored? The folder mprisLabel@moon-0xff.github.com was deleted from .local/share/gnome-shell/extensions/. :thinking:

Moon-0xff commented 3 months ago

Indeed, you installed version 28, but that's because 28 is the version assigned to GNOME 45! We don't set the versions.
And GNOME Shell, GTK, and other stuff uses GSettings to store settings, you need dconf to delete them.

In any case that shouldn't have an effect on this, and as @Batwam mentioned the commit I added should also fix the log-outs from changing the extension index (I forgot both settings triggered the same method!).

We'll need to check if other settings trigger a log-out now.

Note: If it's not clear the version stuff, I'll tell you that the first version I uploaded I assigned it version 10, but extensions.gnome.org reassigned it to 1, and every time I upload to ego the version field is assigned to previous uploaded version + 1, discarding whatever it's on the field. Note2: These two commits reflect the above note: Commit 1, Commit 2

DeN-AlB commented 3 months ago

In any case that shouldn't have an effect on this

So there is no need to remove the extension again and delete all setting, right?

Batwam commented 3 months ago

Correct, no need to delete. The settings can stay and the extension files will be overwritten automatically when a new version is uploaded to extensions.gnome.org.

Please check if you get any crash when changing other settings but I'm pretty sure that there 2 settings would be the ones most likely to cause a crash due to the fact that the label had to be respawned in the bar. You can use the little reset arrows to set the settings back to default easily.

DeN-AlB commented 3 months ago

Thanks for helping! If there is any other issue now, I'll come back here. :smile:

Moon-0xff commented 3 months ago

That's great! Then we can push an update to officially support GNOME 46 right after tackling this.
So, is there's any other setting triggering a log-out?

Batwam commented 3 months ago

It's all looking good for me 👍

Moon-0xff commented 3 months ago

Fix added and GNOME 46 compatibility declared.

I'll upload a new version of the extension to extensions.gnome.org soon enough.

Batwam commented 3 months ago

Sounds good, and thank you to @DeN-AlB for reminding us and raising this new bug!

Moon-0xff commented 3 months ago

Update is live!
If you received the update already you can re-enable the version check if you want:
gsettings set org.gnome.shell disable-user-extensions true

Thanks again @DeN-AlB

DeN-AlB commented 3 months ago

So, is there's any other setting triggering a log-out?

Didn't find a setting that triggers a logout.

you can re-enable the version check if you want

I still have another extension left, that didn't get an update yet. :wink:

Thanks again @DeN-AlB

Thank you both for this great support! :+1: