Re: incorrect use of pidfile(3)

2011/10/13 Dag-Erling Smørgrav <des_at_des.no>:
> Pawel Jakub Dawidek <pjd_at_FreeBSD.org> writes:
>> Dag-Erling Smørgrav <des_at_des.no> writes:
>> > How do we fix this?  My suggestion is to loop until pidfile_open()
>> > succeeds or errno != EAGAIN.  Does anyone have any objections to 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 == 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).  This is
> documented in the man page:
>
>     [EAGAIN]           Some process already holds the lock on the given pid‐
>                        file, but the file is truncated.  Most likely, the
>                        existing daemon is writing new PID into the file.
>
> I have a patch that adds a pidfile to dhclient(8), where I do this:
>
>        for (;;) {
>                pidfile = pidfile_open(path_dhclient_pidfile, 0600, &otherpid);
>                if (pidfile != NULL || errno != EAGAIN)
>                        break;
>                sleep(1);
>        }
>        if (pidfile == NULL) {
>                if (errno == EEXIST)
>                        error("dhclient already running, pid: %d.", otherpid);
>                warning("Cannot open or create pidfile: %m");
>        }
>
> 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.

-- 
"The flames are all long gone, but the pain lingers on"