On Wed, Mar 13, 2013 at 11:18:36AM -0400, John Baldwin wrote:
> On Tuesday, March 12, 2013 4:16:32 pm Dirk Engling wrote:
> > While debugging my own daemon I noticed that pidfile_open does not
> > perform the appropriate checks for a running daemon if the caller does
> > not provide a pidptr to pidfile_open
> >
> > fd = flopen(pfh->pf_path,
> > O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NONBLOCK, mode);
> >
> > fails when another daemon holds the lock and flopen sets errno to
> > EAGAIN, the check 4 lines below in
> >
> > if (errno == EWOULDBLOCK && pidptr != NULL) {
> >
> > means that the pidfile_read is never executed.
> >
> > This results in my second daemon receiving an EAGAIN which clearly was
> > meant to report a race condition between two daemons starting at the
> > same time and the first one not yet finishing pidfile_write.
> >
> > The expected behavior would be to set errno to EEXIST, even if no pidptr
> > was passed.
>
> Yes, I think it should actually perform the same logic even if pidptr is
> NULL of waiting for the other daemon to finish starting up. Something like
> this:
>
> Index: lib/libutil/pidfile.c
> ===================================================================
> --- pidfile.c (revision 248162)
> +++ pidfile.c (working copy)
> @@ -100,6 +100,7 @@ pidfile_open(const char *path, mode_t mode, pid_t
> struct stat sb;
> int error, fd, len, count;
> struct timespec rqtp;
> + pid_t dummy;
>
> pfh = malloc(sizeof(*pfh));
> if (pfh == NULL)
> @@ -126,7 +127,9 @@ pidfile_open(const char *path, mode_t mode, pid_t
> fd = flopen(pfh->pf_path,
> O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NONBLOCK, mode);
> if (fd == -1) {
> - if (errno == EWOULDBLOCK && pidptr != NULL) {
> + if (errno == EWOULDBLOCK) {
> + if (pidptr == NULL)
> + pidptr = &dummy;
> count = 20;
> rqtp.tv_sec = 0;
> rqtp.tv_nsec = 5000000;
I agree EEXIST should be returned, but I don't like reading existing
pidfile (including waiting for the other process to write its PID) just
to throw read PID away.
How about this patch?
http://people.freebsd.org/~pjd/patches/pidfile.c.patch
--
Pawel Jakub Dawidek http://www.wheelsystems.com
FreeBSD committer http://www.FreeBSD.org
Am I Evil? Yes, I Am! http://tupytaj.pl
pgp85B_RZoSMW.pgp
Description: PGP signature
