in reply to Flock Feedback

At the risk of belaboring the points, here's v0.02: 
sub getNewCounter
# ---------------------------------------------------------------
# Reads and incremements the counter value.  Implementation based 
# on wisdom gained from perlmonks.org and the Perl Cookbook.
# ---------------------------------------------------------------
{
   my( $datafile ) = @_;
   my( $lockfile );

   # Determine the name of the lock file
   $_ = $datafile;
   s/\.dat/\.lck/;
   $lockfile = $_;

   # The idea here is to wait up to ten seconds to gain access to 
   # the lock file.  If that fails, well, try again later.

   my( $retries ) = 10;

   open( LOCK, "$lockfile" )  or die "Cannot open lock file for readin
+g. $!";
   while ( $retries )
   {
      if ( flock( LOCK, LOCK_EX | LOCK_NB ) )  # if Fcntl isn't availa
+ble (it should be), try 2 | 4.
      {  
         last;
      } 
      else
      {
         $retries--;
         sleep( 1 ) ;
      }
   }

   # How did we get out of that loop?  If we failed to flock, then fle
+e
   unless ( $retries ) 
      { die "Cannot lock counter for reading. $!"; }

   # Okay, we're assuming here...   
   open( FILE, "+< $datafile" ) or die "Can't open data file for readi
+ng. $!";

   my( $countval ) = <FILE>;
   
   seek( FILE, 0, 0 )           or die "Can't reset data file for writ
+ing. $!";
   truncate( FILE, 0 )          or die "Can't clear data file for writ
+ing. $!";

   print FILE ++$countval;

   close( FILE )                or die "Cannot close counter after wri
+ting. $!";
   close( LOCK )                or die "Cannot close lock file. $!";
   return( $countval );

}

[download]

Yes, it assumes that the sentinel file already exists.

Better? (I ask because I may not be able to get the SecuroTroopers to install File::CounterFile.)

--f

P.S. Thanks to jptxs for picking up on the other question. Also, tilly's newest sub looks interesting and that will go into 0.03, should anything else?

Update: Thanks, tilly for your extreme patience in this. Point by Point:

  1. .dat is fine for this script, but good advice for future ones.
  2. Added LOCK_NB, per advice; see above.
  3. I was under the impression that one second was the finest granularity available for sleep()?
  4. You mean, like the 0.01 did?
  5. *blush*
  6. /me hies himself to a Monastery library...

Update #2: Quickly fixed the || typo and the magic numbers before running off to write v0.03.

Replies are listed 'Best First'.
Re: Re: Flock Feedback
by chipmunk (Parson) on Feb 26, 2001 at 10:02 UTC
            if ( flock( LOCK, 2 || 4 ) ) Down with magic numbers! This code is unnecessarily platform dependent. Use the Fcntl module to get the appropriate constants (as shown in the documentation for flock). (BTW, I'm assuming that the logical instead of bitwise or is just a typo.) 
    use Fcntl ':flock';
# ...
      if ( flock( LOCK, LOCK_EX | LOCK_NB ) )
# ...

    [download]
    P.S. Regarding sleeping... How can I sleep() or alarm() for under a second?
[reply]
[d/l]
[select]
Re (tilly) 2: Flock Feedback
by tilly (Archbishop) on Feb 26, 2001 at 09:50 UTC
    A series of comments:
    1. I would not assume that the data file had a name ending in .dat. That is the kind of assumption that can change, and then people will be unhappy because their data file got wiped out.
    2. You need to set the LOCK_NB flag as well as LOCK_EX or else your first try will hang until you get the lock.
    3. In a CGI environment, waiting 1 second per retry is probably a bit much...
    4. While you have control of the file, you don't need to be cautious about truncates and seeks. Just open for read, read, close, open for write, and write as usual.
    5. Explicitly closing the data file is a good idea.
    6. However I like calling local on *FH if I will use FH as a filehandle. Just a good practice...

    UPDATE
    Others have already said everything that I would have to say other than the fact that my fifth point is that you actually did. I offered you code earlier where the lock went away just by going out of scope. If you use that then you still want to be explicit about closing the date file. I was just saying that it was subtle but important point.

[reply]
Re: Re: Flock Feedback
by dws (Chancellor) on Feb 26, 2001 at 10:17 UTC
        flock( LOCK, 2 || 4 ) Aside from the problem of using magic numbers, this really isn't going to do what you want it to.

    Meditate for a moment on the difference between "|" and "||".

    (Update: the code this note was based on has been fixed above.)

[reply]
[d/l]
Re: Re: Flock Feedback
by tomhukins (Curate) on Feb 26, 2001 at 15:01 UTC

    If you just want a simple incrementing counter, one way to do this is to use the size of the file as the counter, rather than its contents. You don't need locking to do this: 

    my $filename = '/path/to/file';

# Increment the counter (atomically, so no locking required)
open(FILE, ">> $filename") or warn "Missed a count";
print FILE '.'    # Ouput one (arbitrary) byte
close FILE;

# What is the counter set to?
my $countval = -s $filename || 0;

    [download]

    The disadvantage of this technique is that it uses one byte of disk space for each count.

[reply]
[d/l]
      Not only does this use one byte of disk space per count, which is ridiculously wasteful, but you still have a race condition if two processes try to write to the file and then check its size at the same time. Even with this approach you still would need locking.
[reply]

        Even with this approach you still would need locking.

        Yes, as written. But the ">>" file mode does allow you to use something very similar to this without (additional) locking. 

        use IO::Handle;
my $filename = '/path/to/file';

# Increment the counter (atomically, so no locking required)
open(FILE, ">> $filename") or warn "Missed a count";
FILE->autoflush(1);
print FILE '.';    # Ouput one (arbitrary) byte
my $counter= tell(FILE);
close FILE;

        [download]

        Now, if you reset your counters every so often, this can be a really nice little trick. To reset the counters, just do a unlink($filename) once a day/week/month, which also doesn't require any locking on your part! (Though the location and permissions on the counter file are a non-trivial problem so you may want to use sysopen so you can specify permission bits for the case when the file is recreated.)

                - tye (but my friends call me "Tye")
[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom