Date: Thu, 13 Oct 2011 09:21:11 -0300 From: "Carlos A. M. dos Santos" <unixmania@gmail.com> To: =?ISO-8859-1?Q?Dag=2DErling_Sm=F8rgrav?= <des@des.no> Cc: Pawel Jakub Dawidek <pjd@freebsd.org>, current@freebsd.org Subject: Re: incorrect use of pidfile(3) Message-ID: <CAJ4jsadYrZuJ5fMz_YkphVhX%2B5gE0gFOm5ZY_kZ_SRbSz7PkDQ@mail.gmail.com> In-Reply-To: <86botlaz41.fsf@ds4.des.no> References: <86pqi1b1qp.fsf@ds4.des.no> <20111013113024.GE1667@garage.freebsd.pl> <86botlaz41.fsf@ds4.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/10/13 Dag-Erling Sm=C3=B8rgrav <des@des.no>:
> Pawel Jakub Dawidek <pjd@FreeBSD.org> writes:
>> Dag-Erling Sm=C3=B8rgrav <des@des.no> writes:
>> > How do we fix this? =C2=A0My suggestion is to loop until pidfile_open(=
)
>> > succeeds or errno !=3D EAGAIN. =C2=A0Does anyone have any objections t=
o that
>> > approach?
>> I think we already do that internally in pidfile_open(). Can you take a =
look at
>> the source and confirm that this is what you mean?
>
> No, it doesn't; pidfile_open(3) returns NULL with errno =3D=3D EAGAIN if =
the
> pidfile is locked but empty, as is the case in the window between a
> successful pidfile_open(3) and the first pidfile_write(3). =C2=A0This is
> documented in the man page:
>
> =C2=A0 =C2=A0 [EAGAIN] =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Some process al=
ready holds the lock on the given pid=E2=80=90
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0file, but the file is truncated. =C2=A0Most likely, the
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0existing daemon is writing new PID into the file.
>
> I have a patch that adds a pidfile to dhclient(8), where I do this:
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0for (;;) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pidfile =3D pidfil=
e_open(path_dhclient_pidfile, 0600, &otherpid);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pidfile !=3D N=
ULL || errno !=3D EAGAIN)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0break;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sleep(1);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pidfile =3D=3D NULL) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (errno =3D=3D E=
EXIST)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0error("dhclient already running, pid: %d.", otherpid);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0warning("Cannot op=
en or create pidfile: %m");
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> I'm not sure I agree with the common idiom (which I copied here) of
> ignoring all other errors than EEXIST, but that's a different story.
You are also ignoring the return value of sleep(1), which would tell
you if the call was interrupted by a signal handler. This can be fine
for dhclient(8) but other utilities might require some guards against
such interruptions.
--=20
"The flames are all long gone, but the=EF=BB=BF pain lingers on"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ4jsadYrZuJ5fMz_YkphVhX%2B5gE0gFOm5ZY_kZ_SRbSz7PkDQ>