Quodlibet: Current search string is not always restored after a quodlibet restart

[marcelpaulo]

If I quit QL with a string in the search box, and start it back, the search box is populated with an older search string.

Here's an example. Before I quit QL:

Quitting:

After I start it back:

I get the impression that:

1. QL always brings back the last saved search string
2. QL periodically saves the current search string
3. QL doesn't save the current search string when it quits

So, it does seem to be working as designed, and what I see is not a bug, but a request for enhancement.

PS: Using QL 3.3.999+7221~827aa65-0~ppa0~utopic on Xubuntu 14.10

[lazka]

Thanks. I can't reproduce here.. I'll try with Ubuntu.

[marcelpaulo]

Hey, Christoph, I've just observed that actually the current playing song and album aren't also being saved on quitting. Would it be the case of changing the title of this issue to make it more generic, something like _Current running state is not being saved when quitting QL_, instead of creating a separate issue, since both are obviously related ?

EDIT: Running version _3.3.999+7234~9d72926-0~ppa0~utopic_

[marcelpaulo]

I'm now running an updated QL from master (beb4047252188aacc43ae4920ee67189c0dfebd7) on Xubuntu 18.04, and the issue is still present.

To be more precise, I have observed:

1. After #2871, the search string is being saved in lists/queries whenever the entry box loses focus
2. When QL is stopped and restarted, not always the last search string is restored in the entry box
3. If the interval between stopping QL and restarting it is long enough, the last search string is restored in the entry box
4. I haven't discovered where QL gets the search string to restore in the entry box when it restarts
5. I have been stopping QL before quitting it, in the hope the search string would be saved, but stopping it doesn't trigger it. It does seem to be triggered by some other event.

Would be feasible to save the "to be restored after restart" search string at the same time it is saved in lists/queries ?

[frestr]

I can't really reproduce this, but does the following patch fix it? (It's essentially what you're suggesting in your comment.)

diff --git a/quodlibet/quodlibet/qltk/searchbar.py b/quodlibet/quodlibet/qltk/searchbar.py
index 0df39db76..c5714ac76 100644
--- a/quodlibet/quodlibet/qltk/searchbar.py
+++ b/quodlibet/quodlibet/qltk/searchbar.py
@@ -167,6 +167,7 @@ class SearchBarBox(Gtk.HBox):
             self.__combo.prepend_text(text)
             self.__combo.write()
             self.__uninhibit()
+            config.settext("browsers", "query_text", text)

     def __key_pressed(self, entry, event):
         if (is_accel(event, '<Primary>Return') or

[marcelpaulo]

My sincerest apologies, @frestr: I can't reproduce it either, even without applying the patch. I changed the search string, immediately quit QL, restarted it, and the last search string was restored.

Please forgive me for unnecessarily wasting your time.

EDIT: So, I believe your PR https://github.com/quodlibet/quodlibet/pull/2871 killed 2 birds with 1 stone !

[frestr]

No problem. Glad that it's working now :)

[marcelpaulo]

I've just accidentally discovered when the search string is not restored after QL restarts. I've just rebooted my laptop, and when I started QL, the last search string was not restored. So, I guess:

Gracefully quit QL => search string is restored
Kill QL => search string is not restored

I'll apply your patch, @frestr, and check if it makes a difference.

[marcelpaulo]

No joy :-(

Even after applying your patch, if the laptop is rebooted while QL is running, the last search string is not restored when it's started again.

So here's how to reproduce it, I tested it a couple of times:

1. Change the search string
2. Move the focus away from the search entry box so that it's saved
3. Run killall -9 quodlibet
4. Restart QL

[frestr]

I see. QL saves the config vars to disk periodically and when quitting, but when force quitting it doesn't get time to save it. A solution may be to save the config to disk every time there is a change (at least to some variables like query_text), but not I'm not sure how that would affect performance though.

[frestr]

I've looked some more into this, and come up with a possible solution. It works by making a (two second) deferred function call (as to not hog the disk) that writes the config to disk each time the Config object's set method is called.

It's not exactly pretty, and it only saves the config file, i.e. not files like queue, queries etc. These files are not connected to the config, but has in common that they are written on a destroy signal (i.e. when the program closes normally). So all of their corresponding classes will need to be changed in about the same way (i.e. writing to disk on some "changed" signal/event).

Here's the patch if you want to test it, @marcelpaulo. I guess it's a start at least.

diff --git a/quodlibet/quodlibet/util/config.py b/quodlibet/quodlibet/util/config.py
index 62c8233d9..b0027dae8 100644
--- a/quodlibet/quodlibet/util/config.py
+++ b/quodlibet/quodlibet/util/config.py
@@ -37,7 +37,7 @@ except ImportError:
 from senf import fsnative

 from quodlibet.compat import cBytesIO, PY2, PY3, text_type, StringIO
-from quodlibet.util import list_unique, print_d
+from quodlibet.util import list_unique, print_d, DeferredSignal
 from quodlibet.util.atomic import atomic_save
 from quodlibet.util.string import join_escape, split_escape
 from quodlibet.util.path import mkdir
@@ -61,11 +61,13 @@ class Config(object):
     to set default values.
     """

-    def __init__(self, version=None, _defaults=True):
+    def __init__(self, version=None, _defaults=True, autosave=True):
         """Use read() to read in an existing config file.

         version should be an int starting with 0 that gets incremented if you
         want to register a new upgrade function. If None, upgrade is disabled.
+
+        autosave writes the config to disk when a change is detected
         """

         self._config = ConfigParser(dict_type=_sorted_dict)
@@ -75,6 +77,8 @@ class Config(object):
         self._version = version
         self._loaded_version = None
         self._upgrade_funcs = []
+        self._deferred_save = None
+        self._autosave = autosave

     def _do_upgrade(self, func):
         assert self._loaded_version is not None
@@ -328,6 +332,17 @@ class Config(object):

         try:
             self._config.set(section, option, value)
+            if self._autosave:
+                if not self._deferred_save:
+                    from quodlibet import app, config
+
+                    def save():
+                        app.browser.save()
+                        config.save()
+
+                    self._deferred_save = DeferredSignal(
+                            save, timeout=2000, owner=None)
+                self._deferred_save()
         except NoSectionError:
             if self.defaults and self.defaults.has_section(section):
                 self._config.add_section(section)

[marcelpaulo]

It did the trick ! Thank you so much ! Applied the patch, started QL, changed the search string, waited 2s, sent SIGKILL to QL, restarted it: the search string was restored. Great !

[lazka]

Imho you can expect some things not being saved if you SIGKILL the process

[marcelpaulo]

You're right, @lazka. But then again, if the config files are updated more frequently, the chance of restoring the last QL state increases even with a forceful end.

[lazka]

Sure, we currently save the config every 5 minutes.

[frestr]

Imho you can expect some things not being saved if you SIGKILL the process

I guess the main issue is that this happens when the system is powered off (at least on my system), so it essentially happens every time unless QL is manually closed first.

Sure, we currently save the config every 5 minutes.

Do you suggest we decrease that interval instead?

The periodic config save also doesn't save the browser, so values like the query text and pane selection are not saved. (That can be changed of course.)

[lazka]

I guess the main issue is that this happens when the system is powered off (at least on my system), so it essentially happens every time unless QL is manually closed first.

If we get SIGKILLed on shutdown then something is broken I guess. We should get a SIGTERM and stop controlled.

[declension]

If we get SIGKILLed on shutdown then something is broken I guess. We should get a SIGTERM and stop controlled.

Agreed

[marcelpaulo]

Checking the systemd.kill man page:

Processes will first be terminated via SIGTERM (unless the signal to send is changed via KillSignal=). Optionally, this is immediately followed by a SIGHUP (if enabled with SendSIGHUP=). If then, after a delay (configured via the TimeoutStopSec= option), processes still remain, the termination request is repeated with the SIGKILL signal (unless this is disabled via the SendSIGKILL= option)

I get the impression QL receives a SIGTERM and then a SIGKILL before it has finished its stopping routine.

As it's not started as a systemd service, the systemd.kill mechanism cannot be tailored for it.

[marcelpaulo]

Digging a little bit more, about TimeoutSopSec from systemd.service man page:

Configures the time to wait for stop. If a service is asked to stop, but does not terminate in the specified time, it will be terminated forcibly via SIGTERM, and after another timeout of equal duration with SIGKILL (see KillMode= in systemd.kill(5)). Takes a unit-less value in seconds, or a time span value such as "5min 20s". Pass "infinity" to disable the timeout logic. Defaults to DefaultTimeoutStopSec= from the manager configuration file

paulo:~$ systemctl -p DefaultTimeoutStopUSec show
DefaultTimeoutStopUSec=1min 30s

I'm running Xubuntu 18.04, and QL runs as a child of xfce4-panel:

paulo:~$ pstree -s 3467
systemd───xfce4-panel───panel-1-whisker───quodlibet───13*[{quodlibet}]

but xfce4-panel is not a systemd service, so I don't know which kill rule applies.

[marcelpaulo]

Investigating why QL doesn't save all its config during a system shutdown… Ran quodlibet --debug &>quodlibet.log in 3 different scenarios and here's the terminating sequence:

1. Quit QL:
   D: 29.930: qltk.window.__restore_state: Restore state D: 29.930: qltk.window.__restore_position: Restore position D: 29.931: qltk.window.__restore_size: Restore size D: 29.933: main.before_quit: Saving active browser state D: 29.934: main.before_quit: Shutting down player device 'GStreamer: 1.14.1.0'. D: 29.940: PluginManager.save: Saving plugins: 8 active D: 29.940: PluginManager.enable: Disable 'Theme Switcher' D: 29.995: PluginManager.enable: Disable 'ToggleMenuBar' D: 29.995: PluginManager.enable: Disable 'mpris' D: 30.133: PluginManager.enable: Disable 'queue' D: 30.134: PluginManager.enable: Disable 'Website Search' D: 30.134: PluginManager.enable: Disable 'editplaycount' D: 30.134: PluginManager.enable: Disable 'CustomCommands' D: 30.134: PluginManager.enable: Disable 'include_saved' D: 30.135: _main.quit_gtk: Quit GTK: done. D: 30.818: _main.run: Gtk.main() done. D: 30.819: library.save: Saving all libraries... D: 30.819: SongFileLibrary.save(<libraries.SongFileLibrary object at 0x7ff56dd96d80 (quodlibet+library+libraries+SongFileLibrary at 0x1e16e20)>): Saving contents to '/home/paulo/.config/quodlibet/songs'. D: 40.615: config.save: Writing config... D: 40.658: main.main: Finished shutdown.
2. Logged out from current Xfce session:
   D: 26.303: qltk.idle_handler: Python signal handler activated: SIGHUP D: 26.304: qltk.window.__restore_state: Restore state D: 26.305: qltk.window.__restore_position: Restore position D: 26.305: qltk.window.__restore_size: Restore size D: 26.306: main.before_quit: Saving active browser state D: 26.306: main.before_quit: Shutting down player device 'GStreamer: 1.14.1.0'. D: 26.310: PluginManager.save: Saving plugins: 8 active D: 26.311: PluginManager.enable: Disable 'Theme Switcher' D: 26.418: PluginManager.enable: Disable 'ToggleMenuBar' D: 26.419: PluginManager.enable: Disable 'mpris' D: 26.571: PluginManager.enable: Disable 'queue' D: 26.572: PluginManager.enable: Disable 'Website Search' D: 26.572: PluginManager.enable: Disable 'editplaycount' D: 26.572: PluginManager.enable: Disable 'CustomCommands' D: 26.572: PluginManager.enable: Disable 'include_saved' D: 26.572: _main.quit_gtk: Quit GTK: done. Gdk-Message: 21:59:08.141: quodlibet: Fatal IO error 25 (Inappropriate ioctl for device) on X server :0.0.
3. Shutdown
   D: 26.942: qltk.idle_handler: Python signal handler activated: SIGHUP D: 26.943: qltk.window.__restore_state: Restore state D: 26.943: qltk.window.__restore_position: Restore position D: 26.943: qltk.window.__restore_size: Restore size D: 26.945: main.before_quit: Saving active browser state D: 26.945: main.before_quit: Shutting down player device 'GStreamer: 1.14.1.0'. D: 26.955: PluginManager.save: Saving plugins: 8 active D: 26.955: PluginManager.enable: Disable 'Theme Switcher' D: 27.055: PluginManager.enable: Disable 'ToggleMenuBar' D: 27.056: PluginManager.enable: Disable 'mpris' D: 27.190: PluginManager.enable: Disable 'queue' D: 27.191: PluginManager.enable: Disable 'Website Search' D: 27.191: PluginManager.enable: Disable 'editplaycount' D: 27.191: PluginManager.enable: Disable 'CustomCommands' D: 27.191: PluginManager.enable: Disable 'include_saved' D: 27.192: _main.quit_gtk: Quit GTK: done. D: 27.913: qltk.idle_handler: Python signal handler activated: SIGHUP D: 27.914: qltk.idle_handler: Python signal handler activated: SIGTERM Gdk-Message: 22:00:47.182: quodlibet: Fatal IO error 25 (Inappropriate ioctl for device) on X server :0.0.

For QL to complete its shutdown tasks, saving all config, it should save all config before running any commands that access the X server. I don't know if this is feasible, but it seems to be the only way to ensure that QL can cleanly quit on session logout or system shutdown.

If we get SIGKILLed on shutdown then something is broken I guess. We should get a SIGTERM and stop controlled.

@lazka, QL does get a SIGHUP and a SIGTERM and starts to stop in a controlled manner, but in parallel the X server is stopped, so QL aborts before it completes its controlled shutdown.

[lazka]

@marcelpaulo interesting, thanks

[marcelpaulo]

@lazka, I'm wondering if it's the case of closing this issue and opening a more generic one, requesting change in the QL shutdown sequence, so that everything is properly saved on desktop logout or shutdown. That would be the definitive solution to the problem of restarting QL with any outdated information.

[marcelpaulo]

I opened issue #2897 addressing the more generic problem of QL aborting on logout and shutdown without saving anything of its state. I suppose this issue can be closed when #2897 is closed.

[CreamyCookie]

@marcelpaulo Is this still an issue in Quod Libet 4.4 and modern Xfce?

[marcelpaulo]

I have just tested it and the issue is gone. Please forgive me for not having tested it before.

[CreamyCookie]

@marcelpaulo Awesome. Thanks for testing! :+1: