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: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.3-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-19 17:11 UTC by Dave Cottlehuber
Modified: 2016-10-07 12:37 UTC (History)
3 users (show)

See Also:


Attachments
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 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 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 2016-09-22 09:15:50 UTC
Created attachment 175049 [details]
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 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 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.