svn commit: r230869 - head/usr.sbin/daemon

[Pawel Jakub Dawidek]

On Mon, Feb 06, 2012 at 09:39:47AM +0200, Mikolaj Golub wrote:
> 
> On Sun, 5 Feb 2012 22:46:48 +0100 Pawel Jakub Dawidek wrote:
> 
>  PJD> On Sun, Feb 05, 2012 at 11:27:10PM +0200, Mikolaj Golub wrote:
>  >> Ok, using hastd code as a reference :-) here is my implementation.
> 
>  PJD> - I'd not pass selected signals to the child. The parent can still be
>  PJD>   killed with a whole bunch of different signals that are not passed or
>  PJD>   cannot be caught or the child process handle them gracefully.
>  PJD>   Signals should be send to the PID from the pidfile anyway. If someone
>  PJD>   is sending signals to the parent he has no right to expect well
>  PJD>   behaviour from the parent.
> 
> Well, sending a whole bunch of different signals to parent we might not expect
> right behavior, but why not to provide it for the "standard" ones? E.g. on
> shutdown init(8) will send SIGTERM and the daemon will gracefully exit
> terminating the child and cleaning up the pidfile. If the the child process
> does not handle SIGTERM gracefully I don't see much difference from having
> only this one process alive or two (with its monitoring daemon).
> 
> The pidfile is seen in ps(1) output for the daemon process, which allows to
> identify the monitoring daemon with its child. Or we could change its
> proctitle to something like "daemon: cmdname[pid]", similar to what sshd does.
> So people would expect that terminating a daemon will terminate the process it
> monitors.
> 
>  PJD> - Now that we handle the pidfile fully in the parent, I'd move dropping
>  PJD>   provileges after fork(2) and pidfile_write(3). This way pidfiles will
>  PJD>   always be created with root privileges and we can forget about all the
>  PJD>   mess with pid directories, etc.
> 
>  PJD> - With the above you can wait for child to exit with simple wait(2).
> 
> Yes, it looks like much simpler, see the attached patch. But I don't think I
> like it much as it still looks like a half measure to me.

I like this approach much better, as it is just simpler, but it is your
call, Mikolaj.

For the patch itself.

You don't have to have two separate cases depending on request for
pidfile. You can specify NULL pfh to the pidfile functions.
Even in example from the manual page when pfh is NULL there is a case
where we warn, but continue execution and call pidfile functions.
This should simplify the code.

If you do that (actually even if you don't), remember to either use
warn(3) before pidfile_remove(3) and exit(3) after or preserve errno
before calling pidfile_remove(3), as pidfile_remove(3) can modify it if
unlink(2) is unsuccessful or pfh is NULL.

-- 
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20120206/814cde93/attachment.pgp