THere is, would you believe, a module called File::CounterFile that does that very thing

/J\

However, if you post your counter code here, we may be able to figure out why it doesn't work.

Yes. That is exactly what I do (and that's what I've learned from books) - that a file must first be opened and then flock can be called. If this is not the proper way of using flock, what is?

$completeadd = "counter.txt";
open(MFILE, "$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
@filedata1=<MFILE>;
chomp @filedata1;
close(MFILE);
if ($filedata1[0])
{
  $filedata1[0]=$filedata1[0] + 1;
}
else
{
  $filedata1[0] = 1;
}
open(MFILE, ">$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
print MFILE "$filedata1[0]";
close(MFILE);
[download]

open(MFILE, ">$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
[download]

Yup, that's the culprit. You are zeroing the file before obtaining the flock, thus possibly stomping on someone else's read or write.

Either use File::CounterFile, or read my columns that use flock for working examples.

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.

What's wrong with my code?

You have several race conditions. You open the file. You have yet to apply the lock. Then you lock it. You read it then CLOSE THE FILE RELEASING THE LOCK BUT YOU HAVE NOT FINISHED! You then do stuff ;-) While you are doing this stuff any other program can open and lock the file.

There are many solutions but in essence you need a 'lock' that persists until you have completed the update. Ie LOCK, read, increment, write, UNLOCK. This is a rather well worn chestnut. Could I suggest Super Search Gotta go. I have a boarding call....

cheers

tachyon

Because you still have a race condition. KM has written a nice article about this.

Cheers,
Ovid

New address of my CGI Course.

Take a look at the perlopentut man page (perlopentut on CPAN if perldoc is still down). Look in the section 'File Locking', there is example code for what you are trying to do.

You'll want something like this:

#!/usr/bin/perl -w
$| = 1;

use strict;
use Fcntl qw(:flock);

my $count_file = 'counter.txt';

open( my $fh, '+<', $count_file ) or die( "open failed: $!" );
flock( $fh, LOCK_EX ) or die( "flock failed: $!" );

++ ( my $count = <$fh> );

seek( $fh, 0, 0 ) or die( "seek failed: $!" );
truncate( $fh, 0 ) or die( "truncate failed: $!" );

print $fh $count;

close( $fh ) or die( "close failed: $!" );
[download]

$completeadd = "semaphore.txt";
open(SEM, "$completeadd") || die "sacrificial file open failed: $!\n";
flock(SEM, LOCK_EX) || die "sacrificial Lock failed: $!";

$completeadd = "counter.txt";
open(MFILE, "$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
@filedata1=<MFILE>;
chomp @filedata1;
close(MFILE);
if ($filedata1[0])
{
  $filedata1[0]=$filedata1[0] + 1;
}
else
{
  $filedata1[0] = 1;
}
open(MFILE, ">$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
print MFILE "$filedata1[0]";
close(MFILE);

close(SEM);
[download]

Are you use-ing strict, warnings, and Fcntl ':flock' ? If you call flock while LOCK_EX is seen as undef or zero (in numeric context), you won't get the effect you want, and the semaphore will indeed fail silently.

$ perl -e'print +LOCK_EX, $/'
LOCK_EX
$ perl -e'print 0+LOCK_EX, $/
0
$ perl -Mwarnings -e'print 0+LOCK_EX, $/'
Argument "LOCK_EX" isn't numeric in addition (+) at -e line 1.
0
$ perl -Mstrict -Mwarnings -e'print 0+LOCK_EX, $/'
Bareword "LOCK_EX" not allowed while "strict subs" in use at -e line 1
+.
Execution of -e aborted due to compilation errors.
$ perl -Mstrict -Mwarnings -MFcntl=:flock -e'print 0+LOCK_EX, $/'
2
$
[download]

After Compline,
Zaxo

To expand a little on what Zaxo said...

Perl generaly tries to be a very helpful language. By default, it lets you write some strings without quoting them, simply by saying something like print string;. Unfornatly, there's no way to tell this apart from something that you meant to be a function, but never defined, and called without parenthesies. That is, when you write print foo;, you may have meant print "foo";, or you may have meant print foo();. By default, perl thinks you meant the second if a subroutine named foo actualy exists, and the first if there is no such subroutine.

However, this can often create difficult-to-debug problems, and isn't really that helpful in the first place (typing the quotes isn't that hard, and makes your code easier to read). Thus, putting a use strict; at the beginning of your code disables this feature -- print foo; means to run the sub named foo, and print the result. If there is no sub named foo defined when the code is run, then it's an error.

(use strict also changes other things that make it more difficult to cause yourself problems -- it makes you declare your variables, and it keeps you from talking about variables by name when what you really meant was to refer to them -- don't worry about that last. use warnings, on the other hand, warns you when what you said is most likely not what you meant.

Using both of them, as a matter of habit, will make it more obvious when you're messing up, and how, so you can help yourself.

This looks an awful lot like two previously discussed threads. Please see Safe Counter and Safe Counter Follow Up for more information. Reusing an existing working module is likely your best option. See the File::CounterFile module (which is mentioned in the other threads.)

$completeadd = "sacrifice.txt";
open(SAC, "$completeadd") || die "sacrificial file open failed: $!\n";
flock(SAC, LOCK_EX) || die "sacrificial Lock failed: $!";

$completeadd = "counter.txt";
open(MFILE, "$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
@filedata1=<MFILE>;
chomp @filedata1;
close(MFILE);
if ($filedata1[0])
{
  $filedata1[0]=$filedata1[0] + 1;
}
else
{
  $filedata1[0] = 1;
}
open(MFILE, ">$completeadd") || die "file open failed: $!\n";
flock(MFILE, LOCK_EX) || die "Lock failed: $!";
print MFILE "$filedata1[0]";
close(MFILE);

close(SAC);
[download]

I've pretty much just taken peoples' word for the following statement, but it has worked out ok so far: File writes in append mode are atomic for most operating systems.

That being the case, one thing you could do is to, instead of keeping a count, keep a tally. Open the file for append, and add "HIT\n".

Once a day (adjust to taste) a cron job could then be used to add up the tallies. The cron job's script would add all the tallies, plus a number you've stored in the first line of this file. Then rewrite the file (using a temp file) with only the total number at the top. rename() the temp file back over the original, and you've now got a fresh file.

For example:

• At midnight the job runs and resets the file to look like this:
  

  234428\n

  
  
• By 2:00am the file has accumulated some hits and looks like this:
  

  234428\n
  HIT\n
  HIT\n
  HIT\n
  HIT\n
  [download]

  
  
• At the following midnight the cron job runs again and adds up all "HIT\n"s. Let's say there were 1000. So it writes a temp file that looks like:
  

  235428\n

  
  It then renames the tempfile over the original file, and starts a new day.

Kind of elaborate, but if your existing course of action isn't working out for you this may be an alternative, and it's certanly less prone to race conditions. You won't have accurate up to the minute stats, but you can have up to date stats as often as you set your job to run.




Dave

You have a race condition [but see update below] by closing and dropping the lock between reading and writing the file. Do both within the same lock with the '+<' mode of open.

$completeadd = "counter.txt";
open MFILE, '+<', $completeadd or die $!;
flock(MFILE, LOCK_EX) || die $!;
chomp($filedata=<MFILE>);
$filedata++;
seek MFILE, 0, 0;
print MFILE $filedata, $/ or die $!;
close MFILE or die $!;
[download]

You seem to be trying to set up the sacrifice file as a semaphore, but once in place you don't use it for anything.

Update: Oops, now I see the semaphore ought to be effective. Your symptoms indicate that it isn't, however. Is the counter opened elsewhere by something that doesn't honor the semaphore?? Thanks, ++davido.

After Compline,
Zaxo

Nope. This is the only place where the counter is opened. Could it be that the semaphore isn't working?