Flock Subroutine

[mt2k]

I couldn't really decide where to put this, but I figured Meditations would be best.
I was working with a script yesterday and realized that flock() doesn't do much by itself.
Using a simple flock FILEHANDLE, LOCK_EX; won't always succeed in locking a file.
So, I created a subroutine that attempts to lock a file several times, and if it cannot, there is definitely something wrong, and the user is presented with an error.

So here is the small subroutine:

sub lockfile {
my $file = pop();
for $i (1 .. 10) {
sleep .15;
$locked = 1 if (flock $file, LOCK_EX);
}
unless ($locked == 1) { print "\n\nFlock error: could not lock $file";
+ exit; }
}

There. Very easy subroutine that will try flocking a file 10 times before informing the user of an error.
I surprised myself by using pop()!
Anyway, this code is very easy to use:
lockfile (FILEHANDLE);

I could have also allowed an optional parameter that would allow the user to pass what kind of lock to use, rather than simply applying LOCK_EX, but I always use LOCK_EX, so...

Better not be an error in that post... ;-)

[tye]

Congratulations. That is a handy function. Here are some suggestions.

for(0..100) {
    sleep .15;
}

In my machine, this takes nowhere near 15 seconds to run. Why? Because it is identical to:

for(0..100) {
    sleep 0;
}

The standard FAQs have information on how to sleep for less than 1 second. Now that Perl worked around a "bug" in Win32 select(), your best bet is:

    my( $x, $y, $z );
    select($x=undef,$y=undef,$z=undef,0.15);

You don't exit the loop once the flock succeeds. I'd just to a return 1 if flock... and drop the whole $locked variable.

You might want to get into the habbit of using #!/usr/bin/perl -w and use strict;, since they will probably eventually save you a lot of time debugging.

I'd replace the print...exit part with a call to die since that will allow your callers to trap failure if they want to.

And don't worry too much about errors. We all make errors. That is one of the most common ways to learn new things.

Besides, none of my suggestions point out what I would call "show stoppers".

[redmist]

sleep() actually sleeps a different amount of time depending on the architecture. Word around the campfire is that you can actually use select() to sleep for an exact amount of time...instructions are in Camel V2 but the rationale is not.

redmist
redmist.dyndns.org
[e-mail]

[merlyn]

Thus spoke the mt2k:

Using a simple flock FILEHANDLE, LOCK_EX; won't always succeed in locking a file.

Hmm. I don't know of a time that wouldn't either succeed or fail. Maybe you're thinking of or'ing it with LOCK_NB, in which case your code makes sense. But not as it is.

And next, what the heck is this?

sleep .15;

That's a sleep 0, since sleep() doesn't take a fractional number unless you've called in the right overloading module.

Great idea. Bad code. (And I'll probably get flamed for this post again. So be it.)

-- Randal L. Schwartz, Perl hacker

[chip]

But flock does succeed or fail unequivocally.

If I werel logged in to a system where flock didn't work unless I called it 15 times ... well, I'd log the flock out.

    -- Chip Salzenberg, Free-Floating Agent of Chaos

[KM]

Well, I mention this every time I see flock() being used. You introduce a race condition here. Please take a look at nodes 14137, 14139 and 14140 (I had got cut off so used three nodes, sorry). Personally, I would change the subroutine to use a semaphore (sentinal, or whatever you like to call it) file to avoid any race conditions.

Cheers,
KM

[turnstep]

I guess I still do not understand the basic objection to flock - does it not do what it claims to? My understanding is that the OS ensures that only one process can lock a file at a time. If process #2 changes a file between the time that process #1 opens it and locks it, what harm is done? #1 locks it, and then has #2's changes. As long as all your processes are using flock, I still can't quite see the problem with the race condition. Could you please explain it again? Thanks.

P.S. I will be out of the country, so may not reply for a week, but I am interested in this. :)

[KM]

I don't object to flock() at all. It does what it claims to do, but using it incorrectly can be the problem. A race condition is when you have this type ofrun of events which is somewhat common, especially in older scripts (I have seen this a lot in CGI scripts):
1. Open FH for reading
2. Lock FH
3. Read FH
4. Close FH
5. Re-open FH for writing
6. Lock FH
7. Write to FH
8. Close FH

Here you should be able to see the race. Another process can get an exclusive lock on the FH during the read open (read-only opens don't generally get exclusive locks), and between the close of the read and open of the write. Hence, you can have multiple processes working on the file in a way you do not want which could currupt your data ("Hey! Why is my counter file suddenly blank??").

Consider this flow:


1. Proc A open FH for write (using > which clobbers the file contents)
2. Proc B opens FH for reading (no lock attempt since it won't likely get an exclusive lock granted)
3. Proc A locks FH
4. Proc A works with FH
5. Proc A closes FH

One race concern here is that if another process wants to read the contents of this file, it will get garbage since proc A clobbered the file contents. Having proc B attempt an exclusive lock is futile since they are not generally granted to r/o opens. By using semaphores, you can avoid this situation.

These are just two examples (there is also an issue of hardware physically not being done writing to disk before another process opens the file). A good idea is to write the flow of your locks on a whiteboard and see what would happen if multiple processes are doing that same flow at once (I generally add sleeps at key points to show myself this, like in the example script in node 14140.

I hope this makes more sense, if not let me know.

Cheers,
KM

[turnstep]

To expand a little on what has already been said: flock will wait, until it gets a lock. If it does not get one, it is not likely to the next time you try either. Your code might be useful if it was used for open instead of flock, then, once the open was successful, do the flock. Once. If it's going to fail, it's going to fail here. Also, you should only use LOCK_EX when you want to write to the file, and then do your writing as fast as possible, and unlock it as soon as possible. The problem with LOCK_EX is that it not only locks your file, but creates a bottleneck, as only one process can access the file at a time. With LOCK_SH, many processes can read the file simultaneously, although they cannot write to it.

Also, you might want to write

my $file = shift;

## instead of 

my $file = pop();

That way, if you ever add more parameters, (the kind of lock perhaps?) the filename can remain the first parameter, which reads better and fits in with general layout of most significant to least significant in an argument list (i.e. perl functions always have their optional arguments at the end)