[comp.unix.wizards] Is HDB locking safe?

jr@oglvee.UUCP (Jim Rosenberg) (08/06/90)

I have a program which I need to be mutually exclusive with uuxqt.  I've
employed what I thought was pretty straight forward HDB locking using a lock
file, but wasn't sure how to handle one particular problem, & now I don't see
how HDB can handle it correctly either.  HDB assumes that if the pid recorded
in the lock file no longer corresponds to an active process, the lock file is
defunct and can safely be removed.  I can't for the life of me figure out a
safe way of doing this.  You can tell if there's an active process for the pid
by giving it a kill() with a signal number of 0.  Now suppose you get back
ESRCH for errno and conclude that the process holding the lock is no longer
active.  What do you do?

"Elementary, my dear Watson, you remove the lock file!"  *** NOT SO FAST ***,
Holmes.  To unlink the lock file, the only thing you can supply to a system
call is the *name* of the file.  There is no way (so far as I know) to unlink
by i-number.  There's a narrow window in which another process may be doing
exactly the same thing.  You have no guarantee that the LCK.X file you just
unlinked is in fact the same inode as the one from which you read the pid that
you concluded is no longer active.

Here's an example.  This code is taken from pcomm 1.1, which is hideously out
of date, but I had it lying around; it's a good example of some code written
by somebody who took some care and *thought* he was doing the right thing:

static int
checklock(lockfile)
char *lockfile;
{
	...
	if ((lfd = open(lockfile, 0)) < 0)
		return(0);
	...
	if ((kill(lckpid, 0) == -1) && (errno == ESRCH)) {
		/*
		 * If the kill was unsuccessful due to an ESRCH error,
		 * that means the process is no longer active and the
		 * lock file can be safely removed.
		 */
		unlink(lockfile);
		sleep(1);
		return(1);
	}

In this code there is no guarantee that lfd and lockfile correspond to the
same file at the time of the unlink.

I've wracked my brains trying to think of a safe way to do this, and can't
think of one.  How does HDB do it??  Is HDB lock file handling *in fact
vulnerable* to this narrow window problem?  One thing I thought of was to link
the lockfile to a temp file, stat the temp file before the unlink, stat it
again afterwards; if the link count fails to go down you know you made a beeg
booboo and nuked an active lock file.  But then what?  You can't put the lock
file back -- you don't have a link to it.

Help!
-- 
Jim Rosenberg             #include <disclaimer.h>      --cgh!amanue!oglvee!jr
Oglevee Computer Systems                                        /      /
151 Oglevee Lane, Connellsville, PA 15425                    pitt!  ditka!
INTERNET:  cgh!amanue!oglvee!jr@dsi.com                      /      /

chip@tct.uucp (Chip Salzenberg) (08/07/90)

According to jr@oglvee.UUCP (Jim Rosenberg):
>HDB assumes that if the pid recorded in the lock file no longer
>corresponds to an active process, the lock file is defunct and
>can safely be removed.  I can't for the life of me figure out a
>safe way of doing this.

There isn't one.  Sorry.
-- 
Chip Salzenberg at ComDev/TCT     <chip@tct.uucp>, <uunet!ateng!tct!chip>

trt@rti.rti.org (Thomas Truscott) (08/16/90)

> ... HDB assumes that if the pid recorded
> in the lock file no longer corresponds to an active process, the lock file is
> defunct and can safely be removed.  I can't for the life of me figure out a
> safe way of doing this.

A crucial detail in recovering from a breakdown in the lock protocol
is avoiding a race between two or more processes that are simultaneously
attempting recovery.  Usually a strategic pause is all that is needed,
and as you can see in the HDB code below there is just such a pause.

> static int
> checklock(lockfile)
> char *lockfile;
> {
> 	...
> 	if ((lfd = open(lockfile, 0)) < 0)
> 		return(0);
> 	...
> 	if ((kill(lckpid, 0) == -1) && (errno == ESRCH)) {
> 		/*
> 		 * If the kill was unsuccessful due to an ESRCH error,
> 		 * that means the process is no longer active and the
> 		 * lock file can be safely removed.
> 		 */
> 		unlink(lockfile);
> 		sleep(5);		/* avoid a possible race */
> 		return(1);
> 	}
> 
> In this code there is no guarantee that lfd and lockfile correspond to the
> same file at the time of the unlink.

But there *is* a guarantee -- the "sleep(5);"!!
[I changed the sleep() line to match the one in 4.3 BSD uucp "ulockf.c"]

Consider a process "X" that discovers that the locking
process has terminated.  X unlinks the lockfile,
but then it *pauses* before it attempts to grab the lock for itself
(done by code not shown above).

Now consider scenario #1 for another process "Y":
At nearly the same instant Y discovers
the dead lock, so it also unlinks the lockfile
(of course only one unlink can succeed) and it *also pauses*.
Whenever X and/or Y resume there is no lock present,
so attempts to grab it proceed in the usual way (code not shown above).

Now consider scenario #2 for Y:
Just after X has unlinked the lockfile, Y calls checklock()
and discovers no lock is present.  No problem, it just
attempts to grab the lock in the usual way (code not shown above).
When X awakes from its slumber it will discover that Y has
already grabbed the lock, so X will just have to wait.

The HDB code is nice, but does have flaws:
(a) A "sleep(1);" is not enough to avoid a race on a very busy system.
(b) Lock recovery is obscure, so the sleep() call should be commented.
(c) Protocol breakdown is a bad thing, and should be reported:
	logent(lockfile, "DEAD LOCK");
The 4.3 BSD ulockf.c routine has all of these nice features.

	Tom Truscott

goutier@IRO.UMontreal.CA (Claude Goutier) (08/16/90)

In article <4024@rtifs1.UUCP> trt@rti.rti.org (Thomas Truscott) writes:
>
>A crucial detail in recovering from a breakdown in the lock protocol
>is avoiding a race between two or more processes that are simultaneously
>attempting recovery.  Usually a strategic pause is all that is needed,
>and as you can see in the HDB code below there is just such a pause.
>
> ...
>
>The HDB code is nice, but does have flaws:
>(a) A "sleep(1);" is not enough to avoid a race on a very busy system.
>(b) Lock recovery is obscure, so the sleep() call should be commented.
>(c) Protocol breakdown is a bad thing, and should be reported:
>	logent(lockfile, "DEAD LOCK");
>The 4.3 BSD ulockf.c routine has all of these nice features.
>
>	Tom Truscott

If a sleep(1) is not long enough, why does a sleep(5) is?

If something is not prohibited to happen by construction (read solid
and serious interlock), whatever small the probability of it to happen,
it WILL happen!

One should never try to be smarter than a race condition. The only way
is to use true and solid interlocks (which should be provided in the
kernel and with the cooperation of the hardware). Have you ever programmed
on a machine with a fast CPU and ten peripheral processors all accessing
the same memory at the same time ?
--
 Claude Goutier             Services informatiques, Universite de Montreal
                            C.P. 6128, Succ "A",         Montreal (Quebec)
 goutier@jsp.umontreal.ca   Canada      H3C 3J7

peter@ficc.ferranti.com (Peter da Silva) (08/17/90)

In article <4024@rtifs1.UUCP> trt@rti.rti.org (Thomas Truscott) writes:
> (a) A "sleep(1);" is not enough to avoid a race on a very busy system.

No sleep is ever enough. The system could simply be busier than you ever
imagined. You don't solve a race problem by narrowing the window: try
checking the return value of the "unlink": that's the point of failure.
-- 
Peter da Silva.   `-_-'
+1 713 274 5180.   'U`
peter@ferranti.com (currently not working)
peter@hackercorp.com

jfh@rpp386.cactus.org (John F. Haugh II) (08/17/90)

In article <YZ85N+6@xds13.ferranti.com> peter@ficc.ferranti.com (Peter da Silva) writes:
>In article <4024@rtifs1.UUCP> trt@rti.rti.org (Thomas Truscott) writes:
>> (a) A "sleep(1);" is not enough to avoid a race on a very busy system.
>
>No sleep is ever enough. The system could simply be busier than you ever
>imagined. You don't solve a race problem by narrowing the window: try
>checking the return value of the "unlink": that's the point of failure.

There is a big, fat window between the kill() and the unlink() during
which time the other process and kill() the dead process, unlink() the
stale lock, and creat() the new lock file.  Your process will then
unlink() a lock which has just be created.
-- 
John F. Haugh II                             UUCP: ...!cs.utexas.edu!rpp386!jfh
Ma Bell: (512) 832-8832                           Domain: jfh@rpp386.cactus.org

djs@nimbus3.uucp (Doug) (08/18/90)

In article <YZ85N+6@xds13.ferranti.com> peter@ficc.ferranti.com (Peter da Silva) writes:
>No sleep is ever enough. The system could simply be busier than you ever
>imagined. You don't solve a race problem by narrowing the window: try
>checking the return value of the "unlink": that's the point of failure.

Another problem is that the process ids can wrap around and some 
other very long lived process can have that process id, causing uucp and 
cu to not use that modem.

I don't understand why this technique was even used.  The System V kernal 
provides atomic file locking that is released when the process dies or 
closes the file.  Why wasn't that used?  Was it for portability?
What am I missing?
-- 
Doug Scofea   Email: nimbus3!djs@cis.ohio-state.edu    Phone:+1 614 459-1889

les@chinet.chi.il.us (Leslie Mikesell) (08/18/90)

In article <YZ85N+6@xds13.ferranti.com> peter@ficc.ferranti.com (Peter da Silva) writes:
>> (a) A "sleep(1);" is not enough to avoid a race on a very busy system.

>No sleep is ever enough. The system could simply be busier than you ever
>imagined. You don't solve a race problem by narrowing the window: try
>checking the return value of the "unlink": that's the point of failure.

No, there is no problem if the unlink fails since the creation of a new
lock file is done in such a way that only one process will succeed.  The
problem occurs when a process tests one lockfile and decides that no
current process owns it, but the unlink() instead removes a file that was
just created by another program going through a similar procedure.   The
sleep after the unlink() is intended to give any other processes that 
have read the contents of the old lockfile time to complete their unlink()
call before a valid file is created.

Les Mikesell
  les@chinet.chi.il.us

thurlow@convex.com (Robert Thurlow) (08/19/90)

djs@nimbus3.uucp (Doug) writes:

>I don't understand why this technique was even used.  The System V kernal 
>provides atomic file locking that is released when the process dies or 
>closes the file.  Why wasn't that used?  Was it for portability?
>What am I missing?

Either the fact that there was Unix before there was System V, or the
fact that there is still Unix that isn't System V.

Rob T
--
Rob Thurlow, thurlow@convex.com or thurlow%convex.com@uxc.cso.uiuc.edu
----------------------------------------------------------------------
A geek is a terrible thing to waste.  Help support the 1990 "Get A Life"
Initiative!  Send your contributions now! Operators are standing by!

peter@ficc.ferranti.com (Peter da Silva) (08/21/90)

In article <YZ85N+6@xds13.ferranti.com> peter@ficc.ferranti.com (Peter da Silva) writes:
> No sleep is ever enough. The system could simply be busier than you ever
> imagined. You don't solve a race problem by narrowing the window: try
> checking the return value of the "unlink": that's the point of failure.

In article <18500@rpp386.cactus.org> jfh@rpp386.cactus.org (John F. Haugh II) writes:
> There is a big, fat window between the kill() and the unlink() during
> which time the other process and kill() the dead process, unlink() the
> stale lock, and creat() the new lock file.  Your process will then
> unlink() a lock which has just be created.

You're right. Can you suggest a locking protocol that would work here?
I can't imagine that it could be made robust enough to survive a process
crashing during the locking process, but it should be possible to handle
most cases of programs crashing whle locked.

I guess creating a second lock file to be held while deleting the first
would work.
-- 
Peter da Silva.   `-_-'
+1 713 274 5180.   'U`
peter@ferranti.com (currently not working)
peter@hackercorp.com

dhesi%cirrusl@oliveb.ATC.olivetti.com (Rahul Dhesi) (08/25/90)

>I guess creating a second lock file to be held while deleting the first
>would work.

Our in-house software does file locking, to create a lock file L,
like this:

     exlock(L);
     create  L;
     exunlock(L);

     /* critical section here */

     exlock(L);
     delete L;
     exunlock(L);

The definition of exlock(L) is:

     create a lock file called L|lock, looping with sleep(5) several
     times if necessary;  if creation fails, report a very serious
     error and exit process.

The definition of exunlock(L) is:

     delete L|lock;  if nonexistent or can't be deleted, report
     a very serious error and exit process.

Barring a system or process crash during the very brief interval
that L|lock exists, the above scheme is very robust.
--
Rahul Dhesi <dhesi%cirrusl@oliveb.ATC.olivetti.com>
UUCP:  oliveb!cirrusl!dhesi