swaywm / swayidle

Idle management daemon for Wayland
MIT License
550 stars 50 forks source link

Add optional daemonization #82

Open gdamjan opened 4 years ago

gdamjan commented 4 years ago

If the -f argument is passed to swayidle it will fork and daemonize after initialization. This allows proper ordering of other programs after swayidle is fully running, including Type=forking in systemd units

gdamjan commented 4 years ago

The fork/daemonization seems to disable signal handling for some reason.

gdamjan commented 4 years ago

moving the wl_event_loop_add_signal calls after the do_daemonize call fixes that issue, alas we are then back to racy behaviour, or the pipe games to make the parent only exit when it receives a message from the child :(

boucman commented 4 years ago

if the point of daemonization is systemd, maybe it would be better to adapt swayidle for Type=notiy instead ?

forking is very hard to do correctly, and is not very commonly used because of that (most application that are Type=forking do not do the readyness synchronisation correctly, but still use Type=forkins to tell systemd to monitor the correct process)

gdamjan commented 4 years ago

Type=notify is an improvement, but swayidle don't want that

boucman commented 4 years ago

why not ?

emersion commented 4 years ago

moving the wl_event_loop_add_signal calls after the do_daemonize call fixes that issue

Hmm, why does the daemonization remove the signal listener?

boucman commented 4 years ago

let me reiterate... from systemd's point of view, Type=notify is strictly better than Type=forking. It's easier to code (no need to fork or sychronize the child with the parent) and trivial to implement (one call to libsystemd to add at the end of initialization)

So... why do we need that ? is it for synchronization in sysV scripts ? is it for running from some launchers ?

gdamjan commented 4 years ago

Hmm, why does the daemonization remove the signal listener?

I don't know, I didn't find any documentation about that.

gdamjan commented 4 years ago

reproducer https://gist.github.com/gdamjan/a06602fd5bffc5ae500b59bb07729130

gdamjan commented 4 years ago

reproducer https://gist.github.com/gdamjan/a06602fd5bffc5ae500b59bb07729130

the signalfd man page explains why this doesn't work:

epoll(7) semantics If a process adds (via epoll_ctl(2)) a signalfd file descriptor to an epoll(7) instance, then epoll_wait(2) returns events only for signals sent to that process. In particular, if the process then uses fork(2) to create a child process, then the child will be able to read(2) signals that are sent to it using the signalfd file descrip‐ tor, but epoll_wait(2) will not indicate that the signalfd file descriptor is ready. In this scenario, a possible workaround is that after the fork(2), the child process can close the signalfd file descriptor that it inherited from the parent process and then create another signalfd file descriptor and add it to the epoll instance. Alternatively, the parent and the child could delay creating their (separate) signalfd file descriptors and adding them to the epoll instance until after the call to fork(2).

boucman commented 2 years ago

please have a read of https://man7.org/linux/man-pages/man7/daemon.7.html https://man7.org/linux/man-pages/man3/daemon.3.html

making a daemon is "a bit" more involved than just forking...

gdamjan commented 2 years ago

making a daemon is "a bit" more involved than just forking...

sure, but I don't think that's needed for swayidle.

boucman commented 2 years ago

well, you should at least make sure stdin/stdout are properly handled, wihich implies you have to do the double-fork dance...