You don't check whether a) the open b) the flock c) the close of filehandle FLOCK are successful, you don't check the unlink, so your script just silently ignores any misdeeds and whistles along, and you get no clue about what's going on.

To avoid creating/removing the lockfile (takes time, too), you could just lock the script itself, using the DATA filehandle:

if ($cached == 0) {
  exit unless flock(DATA, LOCK_EX | LOCK_NB);
  ...
  flock(DATA, LOCK_UN);
}
...
__END__
__DATA__
[download]

The LOCK_NB is there so the script doesn't wait until a lock is granted.

I'll assume the lock_retrieve() and lock_nstore() both obtain a lock and then operate on the cachefile.

First off, you have some problematic races. There is one between -e $lockfile and open (LOCK, ">$lockfile"); where multiple script instances can fall through to the non-cached path (and then sequentially update the cache file).

Secondly, I think there's a race between close LOCK and unlink $lockfile. After the close, another process can obtain the lockfile, only for this to be promptly deleted, allowing a third process to grab the lock as well...

Thirdly, the locking will needlessly hinder cached reads when it happens.

Now about the fix. 1) Open the cache-file. 2) Stat the open file. 3) If age is less than 2*thresh, use the cached content. 4) If age is more than 1*thresh, trigger $update_needed. 5) When update is needed, launch the updater, or, if access delays are acceptable, perform the locked-update right there. 6) The updater can LOCK_EX|LOCK_NB the cache file, write a new tmpfile and rename that. Coded this way, the cached fast path needs no locking at all. HtH.

Edit. added LOCK_NB above, probably the simplest way to ensure a single updater is run. flock+fork should be acceptable for running the update.

Thank you! I dropped the .lock file and am now locking the script itself. It appears I can lock it again, with no apparent side effect, if it was locked earlier. Found a neat way of referencing the running script without using DATA but now can't find the site to reference it.

The lock_retrieve and lock_nstore were intended to keep other processes waiting, whilst the updated data is being flushed to the cache file.

Herewith the resulting script snippet:

my $cache_ttl = 60;
my $cachefile = '/tmp/mtapi.'.$router.'.cache';
my %bgp_peer;
my $cached = 0;
my $modified = (stat($cachefile))[9];
open our $lockfile, '<', $0 || die $!;
if (defined $modified) {
  if (($modified >= (time-$cache_ttl)) || (!flock $lockfile, LOCK_EX |
+ LOCK_NB)) {
    if (%bgp_peer = %{lock_retrieve($cachefile)}) { $cached = 1 };
  }
}
if ($cached == 0) {
  if (!flock $lockfile, LOCK_EX | LOCK_NB) { print STDERR "Another pro
+cess is already using MikroTik API.\n"; exit 6 };
  $api->connect();
  $api->login();
  %bgp_peer = $api->get_by_key('/routing/bgp/peer/print', 'name');
  $api->logout();
  lock_nstore \%bgp_peer, $cachefile;
  chmod 0660, $cachefile;
  flock $lockfile, LOCK_UN;
}
[download]

Could someone please confirm that Perl would skip trying to acquire a lock when the first 'if' condition is true?

Should I rather place the test in to a nested condition, completely outside of the first?

if ( (1 == 1) || (!flock $lockfile, LOCK_EX | LOCK_NB) )

PS: Things appear to be working as expected, but I would still like to know, for my own peace of mind.