Fixing the race condition in shlock.c

[Julien ÉLIE]

Hi Berend,

> I have atached my current version of shlock.c. I don't know if the
> mailing list will strip attachements...

The attachments problem was fixed a few months ago.


> I downloaded the latest source from isc.org yesterday and the source
> of shlock.c did not have a fix for the race condition in that version
> either.

All right.  Thanks a lot for your patch.
I attach to this mail a suggestion for the new shlock.c.  Does it look
good to you?

I especially used atol instead of atoi, booleans instead of integers,
snprintf instead of sprintf,
changed
  return (pid > 0) && ((kill(pid, 0) == 0) || (errno != ESRCH));
to
  return (pid > 0) && ((kill(pid, 0) >= 0) || (errno != ESRCH));

changed
    /* Don't write exactly sizeof(pid) bytes, someone might treat it
     * as a binary lock. */
-   (void)sprintf(buff, "%10d\n", pid);
to
+   snprintf(buff, sizeof(buff), "%ld\n", (long) pid);

-- is it right?  I do not understand well your comment.

changed
  if (!ok) {
    syswarn(CANTWRITEPID, tmp, strerror(errno));
-    goto ExitUnlink;
  }
to
+    goto ExitClose;

because fd should be closed, shouldn't it?



I believe your patch also fixes what is currently written in the man page:

BUGS
       shlock assumes that it will not be used in an environment with multiple
       locks/unlocks in a short time (due to a race condition).  That is, shlock
       is intended for daily or hourly jobs.




> Not necessarily. It is pretty difficult to trigger on a single cpu
> (single core) system. It all depends on the scheduling of the
> processes. On my dual core system I get multiple failures almost every
> run. On my old pentium 3 it does run without a failure most of the
> time.

That is why I do not manage to reproduce the bug.


> The race condition is that there is a timewindow between the discovery
> of an invalid lock and the removal of that lockfile.

OK.
Is the attached file fine?

-- 
Julien ÉLIE

« César a vraiment pris les gaulois en grippe ! » (Astérix)
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: shlock.c
URL: <https://lists.isc.org/pipermail/inn-workers/attachments/20081116/94f92641/attachment.c>