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

Add optional daemonization #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

gdamjan
Copy link
Contributor

@gdamjan gdamjan commented Oct 12, 2020

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

main.c Outdated Show resolved Hide resolved
main.c Outdated Show resolved Hide resolved
main.c Outdated Show resolved Hide resolved
@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 12, 2020

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

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 12, 2020

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
Copy link
Contributor

boucman commented Oct 13, 2020

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
Copy link
Contributor Author

gdamjan commented Oct 13, 2020

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

@boucman
Copy link
Contributor

boucman commented Oct 13, 2020

why not ?

@emersion
Copy link
Member

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
Copy link
Contributor

boucman commented Oct 18, 2020

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
Copy link
Contributor Author

gdamjan commented Oct 19, 2020

Hmm, why does the daemonization remove the signal listener?

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

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 21, 2020

@gdamjan
Copy link
Contributor Author

gdamjan commented Oct 22, 2020

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).

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
@boucman
Copy link
Contributor

boucman commented Sep 25, 2022

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
Copy link
Contributor Author

gdamjan commented Sep 26, 2022

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

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

@boucman
Copy link
Contributor

boucman commented Sep 26, 2022

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

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

Successfully merging this pull request may close these issues.

None yet

3 participants