Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

Abigail (26)

Abigail
  (email not shown publicly)

Journal of Abigail (26)

Tuesday July 04, 2006
02:18 PM

Buggy locking code.

[ #30169 ]
I was using the following code to prevent two instances of a program to run at the same time:

  use Fcntl qw [:flock :DEFAULT];
  my $lock_file = "...";

  sysopen my $fh => $lock_file, O_RDWR | O_CREAT
      or die "sysopen: $!";
  exit unless flock $fh => LOCK_EX | LOCK_NB;

  ... Code ...

  END {! -f $lock_file || unlink $lock_file || warn "unlink: $!"}

It turned out to be incorrect. Can you spot the mistake?


List all Journal entries
The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.

Buggy locking code. 12 Comments More | Login | Reply /

 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.

  • Operator precedence.

    • No, operator precedence isn't the problem. The problem hasn't anything to do with syntax.

      • Then I guess I just found a bonus bug for you.

        $ perl -MO=Deparse,-p -e'! -f $lock_file || unlink $lock_file || warn "unlink: $!"'
        ((not -f($lock_file)) or unlink(($lock_file || warn("unlink: $!"))));
        -e syntax OK

        Admittedly, it’s not a fatal issue.

  • "On success, zero is returned. On error, -1 is returned, and errno is set appropriately."

    whoops.

  • A third process will erroneously be allowed to launch after the first of two concurrently launched processes has already exited, despite the second is still hanging around.

    But how did you fix it? I can’t see a correct solution other than just not cleaning up the lockfile.

    • Indeed.

      It's easily fixed - set a variable right after getting the lock, and don't delete the file unless the variable is set.

  • oh hehe.

    END {! -f $lock_file || unlink $lock_file || warn "unlink: $!"}

    or in other words...

    "Let's delete the lock file if it exists when we're ENDing, even if we're doing so because we've detected a previous instance of the program"
  • If the processes are running on the same host, why not lock the DATA filehandle?
    flock(DATA, LOCK_EX|LOCK_NB) or die "Already running.";
    should do it. No need for messy temp files to remember to cleanup.
    • Heh, locking the DATA filehandle is a neat idea.

      But more to the point, who would consider using flock() over NFS? That's scary bad stuff. You're now relying on correct implementation of lockd everywhere, which is not guaranteed. Better to use some form of lockfile, the way that the MTAs & procmail do...

      -Dom

      • Slightly more reliable than creating a lock FILE is creating a lock DIRECTORY. I have no doubt some NFS implementation gets this wrong too, but it's quite hard to create a directory non-atomically.
Stories, comments, journals, and other submissions on use Perl; are Copyright 1998-2006, their respective owners.