Add `--replace` option to replace existing services

[matthijskooijman]

Hamster has two background services (hamster-service.py listening on org.gnome.Hamster dbus and hamster-windows-service.py listening on org.gnome.Hamster.WindowServer dbus) that handle the database access and window spawning currently. Additionally, the GUI also has a org.gnome.Hamster.GUI dbus service that can be used to activate an existing GUI process instead of spawning a new one.

During development, you usually want to test a new version of all these services, but by default the existing running services are used. The README documents you can:

# either
pgrep -af hamster
# and kill them one by one
# or be bold and kill all processes with "hamster" in their command line
pkill -ef hamster
python3 src/hamster-service.py &
python3 src/hamster-cli.py

But that is cumbersome and prone to maybe killing too much.

I propose we add a --replace commandline option (inspiration taken from gnome-shell --replace), that will handle this automatically - tell the existing services to quit, and then start new ones.

For this, the two background dbus services seem to have a Quit method that tells them to stop running, which is convenient and allows very targeted termination.

For the GUI, no such method seems to be present, so maybe we need to resort to either figuring out the pid and killing, or just ignoring the service and spawn a new window (and maybe forcibly take over de dbus name if possible).

The README actually documents that the latter is already done:

Advantage: running uninstalled is detected, and windows are not called via D-Bus, so that all the traces are visible.

But this does not seem to work in practice (just ran hamster from flatpak, then ran ./src/hamster-cli.py which focuses the existing window rather than spawn a new one, and the process returns immediately).

The GUI dbus API also seems to be "experimental" still (as documented here), so I'm not sure if its actually used much. In fact, I'm not quite sure what the API really is, I cannot find any dbus setup code and decorators like with the other services. I suspect that the dbus API is implicitly handled by the Gio code and actions defined here, but I'm not sure if that also gives an automatic "quit" action maybe?

[matthijskooijman]

The GUI dbus API also seems to be "experimental" still (as documented here), so I'm not sure if its actually used much. In fact, I'm not quite sure what the API really is, I cannot find any dbus setup code and decorators like with the other services. I suspect that the dbus API is implicitly handled by the Gio code and actions defined here, but I'm not sure if that also gives an automatic "quit" action maybe?

It seems that this API uses the Gio/GTK actions (which are fairly poorly documented - best overview I found is here. In essence, any actions defined on a gtk.Application object will be called remotely on a running application if you make a second instance of that gtk application object and call activate_action on it.

It seems there is actually a quit action defined here, but it also seems the implementation is broken: https://github.com/projecthamster/hamster/blob/198ebe2a4163129b0cb7e773c257ed48cafa73a4/src/hamster-cli.py#L151-L152

This method is missing an argument (leading to TypeError: Hamster.on_activate_quit() takes from 1 to 2 positional arguments but 3 were given), and if that is fixed, it just calls itself (leading to RecursionError: maximum recursion depth exceeded).

So we should fix that quit action, and then we can use it to clear out any running GUI. This won't work if the running GUI is an older version (like 3.0.3), but then you can always just manually quit the GUI.

[matthijskooijman]

I did a bit more digging in the code to figure out how to implement this (and this comment serves to remind myself if not anything else). One thing I really want is to this reliably, meaning free of race conditions (where the code handling --replace sends a quit message, the existing process quits and then the dbus autostart launches a new one automatically before the --replace process can claim the bus name).

What I've found is that dbus keeps a queue of processes that want to claim a specific name, so the replacing processing can put itself in the queue, tell the existing process to quit and then it can be sure that no other process will be started in the meanwhile (there is still the option that there is already another process in the queue that gets the name, but that seems to be enough of a corner case to not care).

Because of this ordering, this means that every separate service (GUI/cli, storage service and windows service) has to handle replacing itself (as opposed to hamster cli sending quit messages to all services), because the quit messages and claiming the name has to be coordinated.

So, my plan is:

• Give each of the three services/commands its own --replace option that triggers this behavior.
• Give hamster-cli an additional --replace-gui option, that runs all three services with --replace.
• To handle replace:
  1. Claim the bus name
  2. Send a quit message to the existing process (if any)
  3. Wait for the name to be assigned (with a timeout of say 10s)

It seems dbus-python does not handle this queuing perfectly (claiming the name does not return whether it was queued or claimed and there is no builtin "wait for claim" feature), but it looks it is possible to query what connection a name is currently assigned to, so I suggest we just poll that a couple of times per second until the name is claimed by us (as an added bonus, we could resend the quit message if the name is claimed by someone else, which I think would give nicer semantics when multiple replacing processes run at the same time, but also lead to more complicated code that has to remember the previous owner, so maybe just keep it simple), or the timeout expires.

Polling is not super nice, and there is a way to get NameOwnerChanged (or something like that) events from dbus, but handling those probably requires significantly more complex code (probably needs a mainloop running at a point where there is none yet), so I would suggest just sticking to polling - this is just development code that will not be run often anyway.