From owner-freebsd-current@FreeBSD.ORG Wed Mar 13 21:26:16 2013 Return-Path: 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 C937FDA5; Wed, 13 Mar 2013 21:26:16 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (garage.dawidek.net [91.121.88.72]) by mx1.freebsd.org (Postfix) with ESMTP id 8F0A8C22; Wed, 13 Mar 2013 21:26:16 +0000 (UTC) Received: from localhost (89-73-195-149.dynamic.chello.pl [89.73.195.149]) by mail.dawidek.net (Postfix) with ESMTPSA id 7D70E6AF; Wed, 13 Mar 2013 22:23:03 +0100 (CET) Date: Wed, 13 Mar 2013 22:27:50 +0100 From: Pawel Jakub Dawidek To: John Baldwin Subject: Re: pidfile_open incorrectly returns EAGAIN when pidfile is locked Message-ID: <20130313212750.GC1372@garage.freebsd.pl> References: <513F8D20.2050707@erdgeist.org> <201303131118.36811.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ncSAzJYg3Aa9+CRW" Content-Disposition: inline In-Reply-To: <201303131118.36811.jhb@freebsd.org> X-OS: FreeBSD 10.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Dirk Engling , freebsd-current@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Mar 2013 21:26:16 -0000 --ncSAzJYg3Aa9+CRW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 > >=20 > > fd =3D flopen(pfh->pf_path, > > O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NONBLOCK, mode); > >=20 > > fails when another daemon holds the lock and flopen sets errno to > > EAGAIN, the check 4 lines below in > >=20 > > if (errno =3D=3D EWOULDBLOCK && pidptr !=3D NULL) { > >=20 > > means that the pidfile_read is never executed. > >=20 > > 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. > >=20 > > The expected behavior would be to set errno to EEXIST, even if no pidptr > > was passed. >=20 > 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 li= ke > this: >=20 > Index: lib/libutil/pidfile.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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; > =20 > pfh =3D malloc(sizeof(*pfh)); > if (pfh =3D=3D NULL) > @@ -126,7 +127,9 @@ pidfile_open(const char *path, mode_t mode, pid_t > fd =3D flopen(pfh->pf_path, > O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NONBLOCK, mode); > if (fd =3D=3D -1) { > - if (errno =3D=3D EWOULDBLOCK && pidptr !=3D NULL) { > + if (errno =3D=3D EWOULDBLOCK) { > + if (pidptr =3D=3D NULL) > + pidptr =3D &dummy; > count =3D 20; > rqtp.tv_sec =3D 0; > rqtp.tv_nsec =3D 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 --=20 Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://tupytaj.pl --ncSAzJYg3Aa9+CRW Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlFA71YACgkQForvXbEpPzRVuACeNxSf6DagXiOt8Dv5/qaT4vbN /RwAmwaTqTGScNqfgQIk7yOcPuJoNRsp =OQHV -----END PGP SIGNATURE----- --ncSAzJYg3Aa9+CRW--