Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] Allow GUI to wait for a daemon #135

Closed
bbaserdem opened this issue May 7, 2020 · 6 comments · Fixed by #148
Closed

[Feature request] Allow GUI to wait for a daemon #135

bbaserdem opened this issue May 7, 2020 · 6 comments · Fixed by #148

Comments

@bbaserdem
Copy link

So from what i understand; the gui starts a new daemon if it cannot find one already running.

In cases where both of them are launched (my setup has gui on xdg-autostart; and daemon started by systemd) I can see where there might be race conditions on which one starts first.

The idea comes to me from the syncthing daemon-gui interaction; where the gui has couple options;

  • Quit if no daemon is found
  • Launch daemon if it's not running
  • If no daemon is found; wait periodically for the daemon.

I thought it might be a nice feature for the gui to wait for the daemon; instead of potentially having two daemons launch.

@samschott
Copy link
Owner

There should be no race conditions because the daemon process creates a lock file during startup. This file creation is atomic because it is carried out with the O_CREAT and O_EXCL flags as in the open(2) specs. If the lock file already exists, it will raise an error and the daemon process will abort.

Personally, I am using the same setup with systemd + xdg-autostart and have always had the systemd service start before the gui. But this is of course not guaranteed. Apart from preventing race conditions, do you see any other advantages in providing those options?

@peterhoeg
Copy link

Instead of having to come up with a solution to handle the race condition, would you be interested in reconsidering your stance from #53 (comment)?

@samschott
Copy link
Owner

samschott commented May 11, 2020

To be fair, there is one possible race condition: the daemon writes its PID to the lock file. Before this write is completed, a second process may think that the lock file is stale because it does not contain a PID which belongs to a running process.

A better way would be to move from lockfile to fasteners, which uses open file descriptors for locking. When the process crashes, the lock is automatically released and no checks for "stale" lock files are required.

@peterhoeg, I definitely see the elegance in relying on systemd to launch the daemon on-demand. For now, many desktop environments still encourage using xdg-desktop entries instead of systemd to launch GUIs (including Gnome, last time I checked) and the issue of supporting older platforms still exists (I still use CentOS 6 at work). I am also a bit reluctant to take on the required work at the moment...

samschott pushed a commit that referenced this issue May 11, 2020
samschott pushed a commit that referenced this issue May 12, 2020
samschott pushed a commit that referenced this issue May 13, 2020
samschott pushed a commit that referenced this issue May 13, 2020
samschott pushed a commit that referenced this issue May 13, 2020
samschott pushed a commit that referenced this issue May 14, 2020
samschott pushed a commit that referenced this issue May 14, 2020
samschott pushed a commit that referenced this issue May 14, 2020
@peterhoeg
Copy link

@peterhoeg, I definitely see the elegance in relying on systemd to launch the daemon on-demand.

So the socket activation option (which by the way would also work for launchd on mac) would completely solve all the race conditions:

  1. init opens the socket (systemd/launchd) on startup
  2. gui runs, sees the socket and connects to it
  3. init spawns daemon and passes socket to it (afaik, this would be the same code path for both systemd and launch)
  4. gui waits until daemon is running

The launching of the GUI daemon could indeed still be an xdg-desktop entry as it really doesn't matter how it's launched.

If you still want the daemon to run eagerly, that would be done via systemctl/launchctl.

I still use CentOS 6 at work

I fully understand that you need to scratch your own itch, but I'm guessing centos 6 deployments are not exactly your core user base. Additionally, you could move the launching into a 5 line shell script and run that from the .desktop file which then takes care of launching things the right way depending on platform.

I am also a bit reluctant to take on the required work at the moment...

I get it - only so many hours in a day. Maybe not a bad first issue for someone new as it doesn't touch the core syncing logic. Some projects use a "good-first-bug" label for this.

@bbaserdem
Copy link
Author

There should be no race conditions because the daemon process creates a lock file during startup. This file creation is atomic because it is carried out with the O_CREAT and O_EXCL flags as in the open(2) specs. If the lock file already exists, it will raise an error and the daemon process will abort.

For some reason; i did not get a notification for this comment. Apart from addressing race conditions; I personally don't have a personal usecase when this might matter. However; I am bound to using a systemd distribution for my dropbox usage by work restrictions. (I have some other software that don't play well unless it's systemd) In theory; for someone using some other management system, the race condition could become significant. I am always an advocat for things working with AND without systemd. (I have been meaning to migrate to gentoo without systemd and instead use supervisord to launch user level daemons; but I won't be needing dropbox on my personal computer) Right now, I am fine with the current state of things. I never had the race condition actually happen as far as I can tell; but I can imagine it happening. I had that happen between syncthing and syncthing-gtk before I configured the gui to wait for a daemon instead of launching one if it can't attach to one.

Though usually there is a delay before xdg-autostart happens upon logging in so this can serve as a barrier to the aforementioned empty lock file issue. Fasteners seems like a good solution to me. Another possibly easy fix may be for the gui to wait a bit (like a few seconds) before trying to launch it's own daemon. Socket activation seems interesting; though I cannot comment further as I don't really know much about it.

The canonical approach in linux systems, as you mentioned, is using xdg-autostart for gui processes. Systemd is not designed to rule graphical objects; and instead is more focused on containerizing processes. There are multiple things that require hacking systemd to work in a way it's not intended (passing environment variables such as XAUTHORITY and DISPLAY in the case of xorg, where systemd does not load environment variables for security reasons, etc.) I would advice against adapting the gui to run under systemd.

I am not running more than one config; but one other consideration regarding this could be how the gui handles multi accounts.

Obviously we can suggest endless features to the project but I have to also express that I am perfectly happy about the state of the project.

@samschott
Copy link
Owner

Thanks for the insight into systemd! I am not really that familiar with it since I mostly work with macOS.

I will migrate to fasteners for now, which uses fcntl locks on open file descriptors. This seems to be all around cleaner than a regular lock file with a PID written to it. The latter can cause both race conditions when checking for a stale lock file and, if the file does not get cleaned up, it's also possible that another process eventually takes its PID, resulting in false positive locks. Of course, there are ways around both, such as waiting for the PID to be written and saving both the PID and process start time. But those increasingly become workarounds compared to a cleaner solution.

I am not running more than one config; but one other consideration regarding this could be how the gui handles multi accounts.

The gui itself doesn't really handle those: every instance is run with its own config file and does not care about others. However, autostart for both GUI and daemon is config-aware in the sense that it will create xdg-desktop or systemd entries for the current config only. So you can end up with multiple entries for different configs alongside each other.

Obviously we can suggest endless features to the project but I have to also express that I am perfectly happy about the state of the project.

This is nice to hear :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants