Using File::PID for long-running jobs

Fri, Dec 30, 2016

I was working with a perl cron job that has the potential to be long running. I was testing the pidfile-based locking the script was doing and found it was exposed to a potential pile up if a dirty pid file was left on disk from an aborted run. I’d used the file::pid module in the following way after finding similar on stackoverflow You might notice one of the comments does warn about a race condition, but doesn’t specify the fix.

Here’s the problematic code:

 use File::Pid;

 my $pidfile = File::Pid->new( {file => '/var/run/script.pid'} );
 exit if $pidfile->running();
 $pidfile->write();

 # the functional bits
 
 $pidfile->remove();
 exit;

The issue with this is that the constructor, when given only the one parameter (file), defaults to setting its ‘pid’ value by reading the value of the file on disk (if it exists). Here’s the module’s CPAN doc. The relevant bit is under ‘new’ and describes the contructor’s behaviors.

The problem is that if a previous run aborted and didn’t remove its pid file, the next run of the constructor will populate its pid value with the value of the aborted run. Now, when running() is called it always looks for the file on disk. It will compare the pid in the dirty pid file on disk to the process table and find it’s not running… so it will proceed (won’t trigger the exit)- this is as it should be. It’s the write() where things go awry- when write is called it will create a pidfile on disk with the pid value from the constructor (the old, dirty one)… so now we’ve just perpetuated the case where the pid in the pidfile on disk doesn’t match anything on the process table. If your job that just kicked off is long-running, any subsequent runs prior to a successful remove() call will pile up.

Fixing this is really simple, and amounts to adding , pid => $$ to the constructor (my pidfile uses the process id of the currently running process). This way it will write() the correct pid to the file on disk after it gets passed running().

I chose to write it this way:

 use File::Pid;

 my $pidfile = File::Pid->new( { file => '/var/run/script.pid', pid => $$ } );
 exit if $pidfile->running();
 $pidfile->write();

 # the functional bits

 $pidfile->remove();
 exit;