Date: Mon, 06 Feb 2012 09:39:47 +0200 From: Mikolaj Golub <trociny@freebsd.org> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, Guy Helmer <ghelmer@palisadesystems.com>, svn-src-all@FreeBSD.org, Andrey Zonov <andrey@zonov.org>, src-committers@FreeBSD.org Subject: Re: svn commit: r230869 - head/usr.sbin/daemon Message-ID: <86sjiov29o.fsf@in138.ua3> In-Reply-To: <20120205214647.GI30033@garage.freebsd.pl> (Pawel Jakub Dawidek's message of "Sun, 5 Feb 2012 22:46:48 %2B0100") References: <201202011641.q11Gf0j6095461@svn.freebsd.org> <20120204074201.GA1694@garage.freebsd.pl> <4F2CEB1D.10607@zonov.org> <27A0A960-F767-4D2C-BF3E-31F73FBF4E28@palisadesystems.com> <86zkcy5ur9.fsf@kopusha.home.net> <20120205093938.GC30033@garage.freebsd.pl> <86lioh7yz5.fsf@kopusha.home.net> <20120205214647.GI30033@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
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.
--
Mikolaj Golub
--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline; filename=daemon.c.patch
Index: usr.sbin/daemon/daemon.c
===================================================================
--- usr.sbin/daemon/daemon.c (revision 231060)
+++ usr.sbin/daemon/daemon.c (working copy)
@@ -32,6 +32,7 @@
__FBSDID("$FreeBSD$");
#include <sys/param.h>
+#include <sys/wait.h>
#include <err.h>
#include <errno.h>
@@ -49,9 +50,9 @@ int
main(int argc, char *argv[])
{
struct pidfh *pfh = NULL;
- int ch, nochdir, noclose, errcode;
+ int ch, nochdir, noclose, status;
const char *pidfile, *user;
- pid_t otherpid;
+ pid_t otherpid, pid;
nochdir = noclose = 1;
pidfile = user = NULL;
@@ -79,43 +80,61 @@ main(int argc, char *argv[])
if (argc == 0)
usage();
- if (user != NULL)
- restrict_process(user);
+ if (pidfile == NULL) {
+ /*
+ * This is a simple case. Daemonize and exec.
+ */
+ if (daemon(nochdir, noclose) == -1)
+ err(1, NULL);
+ if (user != NULL)
+ restrict_process(user);
+
+ execvp(argv[0], argv);
+
+ /*
+ * execvp() failed -- report the error. The child is
+ * now running, so the exit status doesn't matter.
+ */
+ err(1, "%s", argv[0]);
+ }
+
/*
* Try to open the pidfile before calling daemon(3),
* to be able to report the error intelligently
*/
- if (pidfile) {
- pfh = pidfile_open(pidfile, 0600, &otherpid);
- if (pfh == NULL) {
- if (errno == EEXIST) {
- errx(3, "process already running, pid: %d",
- otherpid);
- }
- err(2, "pidfile ``%s''", pidfile);
+ pfh = pidfile_open(pidfile, 0600, &otherpid);
+ if (pfh == NULL) {
+ if (errno == EEXIST) {
+ errx(3, "process already running, pid: %d",
+ otherpid);
}
+ err(2, "pidfile ``%s''", pidfile);
}
if (daemon(nochdir, noclose) == -1)
err(1, NULL);
- /* Now that we are the child, write out the pid */
- if (pidfile)
+ pid = fork();
+ if (pid == -1) {
+ pidfile_remove(pfh);
+ err(1, "fork");
+ }
+ if (pid == 0) {
+ /* Now that we are the child, write out the pid. */
pidfile_write(pfh);
- execvp(argv[0], argv);
+ if (user != NULL)
+ restrict_process(user);
- /*
- * execvp() failed -- unlink pidfile if any, and
- * report the error
- */
- errcode = errno; /* Preserve errcode -- unlink may reset it */
- if (pidfile)
- pidfile_remove(pfh);
+ execvp(argv[0], argv);
- /* The child is now running, so the exit status doesn't matter. */
- errc(1, errcode, "%s", argv[0]);
+ /* execvp() failed. */
+ err(1, "%s", argv[0]);
+ }
+ (void)wait(&status);
+ pidfile_remove(pfh);
+ exit(0);
}
static void
--=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86sjiov29o.fsf>