From owner-freebsd-current@FreeBSD.ORG  Wed Mar 13 19:20:29 2013
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 by hub.freebsd.org (Postfix) with ESMTP id 583D364C;
 Wed, 13 Mar 2013 19:20:29 +0000 (UTC) (envelope-from jhb@freebsd.org)
Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net
 [IPv6:2001:470:1f10:75::2])
 by mx1.freebsd.org (Postfix) with ESMTP id DD28F124;
 Wed, 13 Mar 2013 19:20:28 +0000 (UTC)
Received: from jhbbsd.localnet (unknown [209.249.190.124])
 by bigwig.baldwin.cx (Postfix) with ESMTPSA id 33AACB91A;
 Wed, 13 Mar 2013 15:20:28 -0400 (EDT)
From: John Baldwin <jhb@freebsd.org>
To: freebsd-current@freebsd.org
Subject: Re: pidfile_open incorrectly returns EAGAIN when pidfile is locked
Date: Wed, 13 Mar 2013 11:18:36 -0400
User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; )
References: <513F8D20.2050707@erdgeist.org>
In-Reply-To: <513F8D20.2050707@erdgeist.org>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Message-Id: <201303131118.36811.jhb@freebsd.org>
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7
 (bigwig.baldwin.cx); Wed, 13 Mar 2013 15:20:28 -0400 (EDT)
Cc: Dirk Engling <erdgeist@erdgeist.org>
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
 <freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-current>, 
 <mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
 <mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 13 Mar 2013 19:20:29 -0000

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