Flocking .l file instead of file to be updated

delirium has asked for the wisdom of the Perl Monks concerning the following question:

I recently completed a project that, while it seems to be working famously, worries me a little. I'm using Parallel::ForkManager and Data::Dumper/do(file) (rather than Storable) to run multiple comm sessions at the same time, and have each fork update a history file (that is really just a hash dump).

I wanted to avoid any possible race conditions when updating the file. The file's content may shrink, so I didn't want to open the file +<. I settled on creating .l files, establishing locks on those, and doing reads and updates on the history file only after establishing locks on the .l file.

My question is: Am I overlooking something dangerous? Access to the history file is restricted, so I'm not too concerned about using do(file) (although though I'll be happy when Storable is installed or Perl is upgraded to 5.8x). I don't see any race conditions that can occur in the code, but I'm still worried if I'm missing something that will occur when more sessions are run simultaneously.

Here is a trimmed down code snippet:

#!/usr/bin/perl -w
use strict;
use Data::Dumper;
use Parallel::ForkManager;

my %sess_hist = ();           # Session history hash
my %hash = ();                # session flow information
my $session;                  # ID of current session
my $hist_file = '~/hist.dat'; # Session history file

## Check for sessions that are due/overdue and run them
for (keys %{$hash{Session}})    {
    $session = $_;
    my $pid = $pm->start($session) and next;
    if (&check_overdue) {
        if (open SESSLOCK, '>', "~/$session.l") {
            my $stime = time;
            flock SESSLOCK, 2;
            &parse_hist;
            if ($stime > $sess_hist{$session}{last}) { &run_session; }
            else { logit "Aborting to avoid simultaneous sessions"; }
            close SESSLOCK;
            unlink "~/$session.l";
        }
        else { logit "Can't open session lock file, aborting"; }      
+      
    }
    exit(0);
    $pm->finish($session);
}
$pm->wait_all_children;
unlink "$hist_file.l";


sub parse_hist  {   # Opens hist file and pulls info into %sess_hist
    if (-s $hist_file)  {                   # If the history file has 
+data,
        local $/ = undef;                   # set slurp mode,
        open HFL, '>', "$hist_file.l";      # History File Lock
        flock HFL, 2;                       # establish an exclusive l
+ock,
        %sess_hist = %{do($hist_file)};     # and load hist file as %s
+ess_hist
        close HFL;
    }
}


sub merge_hist_changes  {
    # Create copy of current session data
    my %temp_hash = %{$sess_hist{$session}};

    # Reload hist file after establishing lock on .l file
    open HFL, '>', "$hist_file.l";
    flock HFL, 2;
    local $/ = undef;
    %sess_hist = %{do($hist_file)} if -s $hist_file;

    # Change reference to point to copy made earlier
    $sess_hist{$session} = \%temp_hash;

    # Dump it
    local $Data::Dumper::Indent = 1;
    open  HF, '>', $hist_file;
    print HF Dumper \%sess_hist;
    close HF;
    close HFL;  # Release lock and let next child update $hist_file
}
[download]

&merge_hist_changes gets called at the end of &run_session to merge any changes already saved by other sessions into the current hash. The $session.l files are just to prevent the same session ID being run twice.

Thanks for any input.

Comment on Flocking .l file instead of file to be updated Download Code

Replies are listed 'Best First'.
Re: Flocking .l file instead of file to be updated by duff (Parson) on Dec 02, 2003 at 15:30 UTC
I didn't look at your code in depth, but I did see something that you probably want to change. `close SESSLOCK; unlink "~/$session.l";` [download] I think you want to switch the order of these two lines as the lock goes away as soon as you close the file, so inbetween the time you close and unlink, `~/$session.l` could be used for nefarious purposes. :-) PerlJam	[reply] [d/l] [select]
Re: Flocking .l file instead of file to be updated by Anonymous Monk on Dec 02, 2003 at 17:11 UTC
You don't want to unlink your lockfiles. In UNIX, it's possible to unlink a file that's opened by another process, so you can get this sequence: Process A creates, opens and locks lockfile Process B opens lockfile, tries to lock it, and blocks Process A unlocks and unlinks lockfile Process B locks the now-unlinked lockfile Process C creates, opens and locks a new lockfile At this point, both B and C think they have exclusive access to the resource. It is very easy to cause this situation to occur. Doing the unlink before the unlock doesn't help; the problem is that B is still trying to lock the old file that A unlinked. Don't unlink lockfiles unless you're into pain. Just leave them there.	[reply]
Re: Flocking .l file instead of file to be updated by Anonymous Monk on Dec 02, 2003 at 17:23 UTC
`flock SESSLOCK, 2;` Error check that flock. You don't want to go on happily thinking the file is locked if the syscall failed for some reason (out of memory? networked filesystem?). For readability, use the LOCK_EX constant from Fcntl.	[reply] [d/l]