Bug 212829 - daemon(8) using -P swallows signals such as SIGHUP instead of propagating them
Summary: daemon(8) using -P swallows signals such as SIGHUP instead of propagating them
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.3-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-19 17:11 UTC by Dave Cottlehuber
Modified: 2023-04-19 11:06 UTC (History)
7 users (show)

See Also:


Attachments
patch to daemon.c v1 (1.92 KB, patch)
2016-09-22 09:15 UTC, Dave Cottlehuber
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Cottlehuber freebsd_committer freebsd_triage 2016-09-19 17:11:00 UTC
When a service such as net-mgmt/riemann (java/clojure) is run under daemon(8), `service reload` sends the SIGHUP to the parent daemon(8) process instead of the managed child process.

command="/usr/sbin/daemon"
command_args="-f -c -r -P ${pidfile} \
    ${riemann_java_home}/bin/java \
    ${riemann_java_opts} ...

If the -p flag is used instead, which would provide the child's PID, then `service restart` restarts the child, and not the parent daemon(8) process, which then tries to restart *another* child. messy.

I can't see a way around this other than either not using daemon(8), or alternatively teaching daemon(8) in -P mode to propagate SIGHUP to the child process.


# notes from irc

21:20 <dch> is it possible for daemon(8) to pass on the SIGHUP it receives via `service $name reload` to the child? it feels like this should be possible somehow
21:24 <@cem> It would be possible
23:06 <RootWyrm> dch: thought it already did.. I might have a kludge that uses child_pidfile somewhat handy.
01:56 <dch> RootWyrm: I thought it did too, I looked at /usr.sbin/daemon/daemon.c and realise I am out of my depth here.
01:58 <dch> AFAICT the SIGHUP is only sent by service(8) to daemon, and daemon doesn't propagate it further, when using `command_args="-f -c -r -P ${pidfile} … `
01:59 <dch> I think I swapped from -p  to -P because using -p causes service(8) to stop/start the child pid, and the daemon process restarts it behind your back.
02:30 <RootWyrm> But yes, should depend on -p.. may need an -s flag to pass signal to -p ?
Comment 1 Phillip R. Jaenke 2016-09-19 22:02:40 UTC
To elaborate on my general thoughts;

The issue with just passing signals through daemon is: what if we want to signal daemon? Whoops. The other possibility is to use uncommonly used signals but then all you've got is SIGUSR1 and SIGUSR2 without violating sanity. Obviously, that is not an acceptable solution either. 
So my thought is that a user who wishes for daemon(8) to pass signals to the child needs to provide two explicit arguments at runtime. First, the existing child pid argument -p. Second, the flag -s 'signal passing.' This would tell daemon to simply pass through all signals verbatim to the PID contained within -p. (-s would conflict with -r as well.) This means it's an explicit user-initiated behavior change, rather than function change.

Obviously, this then presents the problem: how to signal daemon itself. That I do not have an elegant solution for. My initial thought was to have a socket equivalent to -P where the user could simply echo the desired signal number to. e.g. "echo '9' > /var/run/daemon/234.pid" - which isn't exactly optimal either. However, this might be a more workable solution for child signalling perhaps?

Thoughts and feedback definitely appreciated.
Comment 2 Mark Felder freebsd_committer freebsd_triage 2016-09-21 20:03:10 UTC
adding mjg as he is reviewing other daemon related things for me
Comment 3 Dave Cottlehuber freebsd_committer freebsd_triage 2016-09-22 09:15:50 UTC
Created attachment 175049 [details]
patch to daemon.c v1

Thanks prj@. I'm not sure what usage daemon gets outside ports tree, but I assume hooking it up to rc.d framework services is the major use case.

Looking at daemon.c it has a special case to forward SIGTERM, but nothing for SIGHUP so I assume by default SIGHUP does  Something like this? see pretty diff https://git.io/vi7j2
Comment 4 Dave Cottlehuber freebsd_committer freebsd_triage 2016-09-22 09:21:59 UTC
BTW I can see the advantages of daemon accepting all signals but then the change becomes more complex - the original issue was allowing the SIGHUP from `service widget reload` to be passed through. 

Apparently /etc/rc.subr allows passing arbitrary signals through if defined in the widget's script:

                reload)
                        if [ -z "$rc_pid" ]; then
                                _run_rc_notrunning
                                return 1
                        fi

                        _run_rc_precmd || return 1

                        _doit=$(_run_rc_killcmd "${sig_reload:-HUP}")
                        _run_rc_doit "$_doit" || return 1

                        _run_rc_postcmd
                        ;;


viz:

japanese/ebnetd/files/ebhttpd.in:sig_reload=SIGHUP
japanese/ebnetd/files/ebnetd.in:sig_reload=SIGHUP
japanese/ebnetd/files/ndtpd.in:sig_reload=SIGHUP
lang/php55/files/php-fpm.in:sig_reload="USR2"
lang/php56/files/php-fpm.in:sig_reload="USR2"
lang/php70/files/php-fpm.in:sig_reload="USR2"
mail/mimedefang/files/patch-examples__init-script.in:     sig_reload="INT"
mail/opendkim/files/milter-opendkim.in:sig_reload="USR1"
mail/rspamd-devel/files/rspamd_redirector.in:sig_reload="USR1"
mail/rspamd/files/rspamd.in:sig_reload="HUP"
mail/rspamd/files/rspamd_redirector.in:sig_reload="USR1"
net-mgmt/icinga-core/files/icinga.in:sig_reload=HUP
net-mgmt/icinga2/files/icinga2.in:sig_reload=HUP
net-mgmt/nagios/files/nagios.in:sig_reload=HUP
net-mgmt/nagios4/files/nagios.in:sig_reload=HUP
net-mgmt/nrpe/files/nrpe2.in:sig_reload=HUP
net/exabgp/files/exabgp.in:sig_reload="USR1"
net/exaddos/files/exaddos.in:sig_reload="USR1"
net/vncreflector/files/vncreflector.in:sig_reload=USR2
security/openvpn-devel/files/openvpn.in:    sig_reload=USR1 run_rc_command reload
security/openvpn/files/openvpn.in:    sig_reload=USR1 run_rc_command reload
security/openvpn/files/openvpn.in:      sig_reload=USR2
sysutils/burp/files/burp.in:sig_reload="HUP"
sysutils/sec/files/sec.in:sig_reload=HUP
www/kannel-sqlbox/files/kannel_sqlbox.in:sig_reload=SIGUSR1
www/kannel/files/kannel_bearerbox.in:sig_reload=SIGUSR1
www/kannel/files/kannel_smsbox.in:sig_reload=SIGUSR1
www/kannel/files/kannel_wapbox.in:sig_reload=SIGUSR1
www/lighttpd/files/lighttpd.in: sig_reload="INT"
www/nghttp2/files/nghttpx.in:sig_reload="USR2"          # exec-self reload; Note: future versions have SIGHUP to reload
www/nghttp2/files/nghttpx.in:   sig_reload="USR1"
www/nginx-devel/files/nginx.in: sig_reload="USR2"
www/nginx-devel/files/nginx.in: sig_reload="QUIT"
www/nginx/files/nginx.in:       sig_reload="USR2"
www/nginx/files/nginx.in:       sig_reload="QUIT"
www/ufdbguard/files/ufdbguardd.in:      sig_reload=USR1
www/ufdbguard/files/ufdbguardd.in:      sig_reload=USR2
www/uwsgi/files/uwsgi.in:       sig_reload="TERM"

TLDR a generalised solution might be better.
Comment 5 Phillip R. Jaenke 2016-09-22 13:32:59 UTC
Yes; after some discussion with feld@ I actually realized that this issue is the actual root cause of problems I've been having with socat in my own environment. The 'proper' way to terminate socat is SIGKILL, not SIGTERM. Right now it sounds like daemon(8) isn't even working as intended and desired; now we've got cases that are exposing further shortcomings. (And no, I'm not using rc.subr.) 

What's supposed to happen is that when I issue a SIGKILL to daemon(8) it is supposed to pass that SIGKILL through to the child immediately, then terminate itself. Basically any signal should be passed through verbatim, then handled by daemon(8) itself. That doesn't appear to be what's happening; at least not reliably. So, it seems to be a mix of bug and shortcoming.

This definitely warrants some further discussion to see what the preferred resolution to both issues would be.
Comment 6 Dave Cottlehuber freebsd_committer freebsd_triage 2016-10-07 12:37:00 UTC
prj@ I agree that the desired behaviour in most cases is likely to be
to pass the signal on (possibly referencing some config or masking list)
and then handle the signal in daemon as required.

# workaround

/bin/pkill -HUP -U riemann java

works for me.
Comment 7 Ihor Antonov 2023-03-06 04:18:06 UTC
I am working on daemon improvements and this feature is planned to be added once transition to kqueue is finished.
Comment 8 Ihor Antonov 2023-04-15 00:22:21 UTC
After reading all comments I take the side of Phillip R. Jaenke (prj)

Different services expect different signals, so creating a mechanism to pass signals to a child leaves a question "how to signal daemon" open. It is obviously a bad design.

The ideal solution would be to have a socket that daemon listens for various commands.
I need to run this idea by kevans to make sure he has no objections for this work.
This might or might not be considered a scope creep. And if it is - I am going to spin off a new supervisor daemon. With sockets, config files, blackjack and hookers :)


In the meanwhile It is possible make `service` to send signal to the child.
There are 2 ways:

1) In your rc script

    pidfile=/path/to/child/pid
    command="/usr/sbin/daemon"
    command_args="--child-pidfile $pidfile ....

rc framework on `service foo reload` should send SIGHUP to the process identified by $pidfile

2) There is a way to add custom commands to the rc script:

    extra_commands="foobarnate"
    pidfile=/path/to/child/pid
    command="/usr/sbin/daemon"
    command_args="--child-pidfile $pidfile ....

    myservice_foobarnate() {
        kill -SIGUSR1 $pidfile
    }

So you can run `service myservice foobarnate` to execute completely custom logic.
Comment 9 Ihor Antonov 2023-04-15 00:36:05 UTC
See /etc/rc.d/ipfilter for a concrete example of custom commands
Comment 10 Ihor Antonov 2023-04-15 13:57:30 UTC
Ok, after talking to kevans@ we are not going to be adding a socket to daemon. So this is a WONTFIX.
Comment 11 crest 2023-04-19 11:06:33 UTC
There are several process supervisors available in ports that could be used instead of daemon. It should be enough to declare a runtime dependency on it and use it instead of daemon in your rc.d script(s). The better ones e.g. s6/runit/daemontools support sending signals to the supervised processes and can capture their stdout/stderr into a pipe connected to a logging process this can be a proxy to syslog (or something better).