Date: Wed, 13 Mar 2013 11:18:36 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org Cc: Dirk Engling <erdgeist@erdgeist.org> Subject: Re: pidfile_open incorrectly returns EAGAIN when pidfile is locked Message-ID: <201303131118.36811.jhb@freebsd.org> In-Reply-To: <513F8D20.2050707@erdgeist.org> References: <513F8D20.2050707@erdgeist.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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; -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201303131118.36811.jhb>