From 5a8252efc53d5bf3e86deeb1021fa2411ab0a4eb Mon Sep 17 00:00:00 2001
From: Agustin Moyano <agustinmoyano@theke.io>
Date: Thu, 5 Dec 2019 18:59:17 -0300
Subject: [PATCH] Bug 23571: Add flock to fines.pl

This patch adds a lock file for fines.pl.

To test:
1. Apply this patch
2. In two separate consoles run misc/cronjobs/fines.pl
SUCCESS => The first one runs normally, but the second one fails with a message flock failed for the lock file.
3. Sign off.

Sponsored-by: Orex Digital
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 misc/cronjobs/fines.pl | 180 ++++++++++++++++++++++++++---------------
 1 file changed, 117 insertions(+), 63 deletions(-)

diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl
index aa9738a01d..2505e2f3a0 100755
--- a/misc/cronjobs/fines.pl
+++ b/misc/cronjobs/fines.pl
@@ -36,16 +36,20 @@ use C4::Overdues;
 use Getopt::Long;
 use Carp;
 use File::Spec;
+use Fcntl qw(:flock);
 
 use Koha::Calendar;
 use Koha::DateUtils;
 use C4::Log;
 
+use constant LOCK_FILENAME => 'fines..LCK';
+
 my $help;
 my $verbose;
 my $output_dir;
 my $log;
 my $maxdays;
+my $use_flock = 1;
 
 GetOptions(
     'h|help'    => \$help,
@@ -77,6 +81,26 @@ if ($help) {
 
 cronlogaction();
 
+my ($lockfile, $LockFH);
+foreach (
+    '/var/lock/koha',
+    '/var/lock/koha_fines',
+    '/tmp/koha_fines'
+) {
+    #we try three possibilities (we really want to lock :)
+    next if !$_;
+    ($LockFH, $lockfile) = _create_lockfile($_);
+    last if defined $LockFH;
+}
+if( !defined $LockFH ) {
+    print "WARNING: Could not create lock file $lockfile: $!\n";
+    print "Verify if directories /var/lock/koha/". C4::Context->config('database').", /var/lock/koha_fines_" . C4::Context->config('database')." or /tmp/koha_fines_" . C4::Context->config('database')." exist and check file permissions\n";
+    $use_flock = 0; # we disable file locking now and will continue
+                    # without it
+                    # note that this mimics old behavior (before we used
+                    # the lockfile)
+};
+
 my @borrower_fields =
   qw(cardnumber categorycode surname firstname email phone address citystate);
 my @item_fields  = qw(itemnumber barcode date_due);
@@ -103,78 +127,83 @@ my $counted = 0;
 my $params;
 $params->{maximumdays} = $maxdays if $maxdays;
 my $overdues = Getoverdues($params);
-for my $overdue ( @{$overdues} ) {
-    next if $overdue->{itemlost};
+if(_flock($LockFH, LOCK_EX|LOCK_NB)) {
+    for my $overdue ( @{$overdues} ) {
+        next if $overdue->{itemlost};
 
-    if ( !defined $overdue->{borrowernumber} ) {
-        carp
-"ERROR in Getoverdues : issues.borrowernumber IS NULL.  Repair 'issues' table now!  Skipping record.\n";
-        next;
-    }
-    my $borrower = BorType( $overdue->{borrowernumber} );
-    my $branchcode =
-        ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch}
-      : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
-      :                                     $overdue->{branchcode};
-
-# In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here).
-    if ( !exists $is_holiday{$branchcode} ) {
-        $is_holiday{$branchcode} = set_holiday( $branchcode, $today );
-    }
+        if ( !defined $overdue->{borrowernumber} ) {
+            carp
+    "ERROR in Getoverdues : issues.borrowernumber IS NULL.  Repair 'issues' table now!  Skipping record.\n";
+            next;
+        }
+        my $borrower = BorType( $overdue->{borrowernumber} );
+        my $branchcode =
+            ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch}
+        : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
+        :                                     $overdue->{branchcode};
 
-    my $datedue = dt_from_string( $overdue->{date_due} );
-    if ( DateTime->compare( $datedue, $today ) == 1 ) {
-        next;    # not overdue
-    }
-    ++$counted;
-
-    my ( $amount, $unitcounttotal, $unitcount ) =
-      CalcFine( $overdue, $borrower->{categorycode},
-        $branchcode, $datedue, $today );
-
-    # Don't update the fine if today is a holiday.
-    # This ensures that dropbox mode will remove the correct amount of fine.
-    if ( $mode eq 'production' && !$is_holiday{$branchcode} ) {
-        if ( $amount && $amount > 0 ) {
-            UpdateFine(
-                {
-                    issue_id       => $overdue->{issue_id},
-                    itemnumber     => $overdue->{itemnumber},
-                    borrowernumber => $overdue->{borrowernumber},
-                    amount         => $amount,
-                    due            => output_pref($datedue),
-                }
-            );
+    # In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here).
+        if ( !exists $is_holiday{$branchcode} ) {
+            $is_holiday{$branchcode} = set_holiday( $branchcode, $today );
+        }
+
+        my $datedue = dt_from_string( $overdue->{date_due} );
+        if ( DateTime->compare( $datedue, $today ) == 1 ) {
+            next;    # not overdue
+        }
+        ++$counted;
+
+        my ( $amount, $unitcounttotal, $unitcount ) =
+        CalcFine( $overdue, $borrower->{categorycode},
+            $branchcode, $datedue, $today );
+
+        # Don't update the fine if today is a holiday.
+        # This ensures that dropbox mode will remove the correct amount of fine.
+        if ( $mode eq 'production' && !$is_holiday{$branchcode} ) {
+            if ( $amount && $amount > 0 ) {
+                UpdateFine(
+                    {
+                        issue_id       => $overdue->{issue_id},
+                        itemnumber     => $overdue->{itemnumber},
+                        borrowernumber => $overdue->{borrowernumber},
+                        amount         => $amount,
+                        due            => output_pref($datedue),
+                    }
+                );
+            }
+        }
+        if ($filename) {
+            my @cells;
+            push @cells,
+            map { defined $borrower->{$_} ? $borrower->{$_} : q{} }
+            @borrower_fields;
+            push @cells, map { $overdue->{$_} } @item_fields;
+            push @cells, $unitcounttotal, $amount;
+            say {$fh} join $delim, @cells;
         }
     }
-    if ($filename) {
-        my @cells;
-        push @cells,
-          map { defined $borrower->{$_} ? $borrower->{$_} : q{} }
-          @borrower_fields;
-        push @cells, map { $overdue->{$_} } @item_fields;
-        push @cells, $unitcounttotal, $amount;
-        say {$fh} join $delim, @cells;
+    if ($filename){
+        close $fh;
     }
-}
-if ($filename){
-    close $fh;
-}
 
-if ($verbose) {
-    my $overdue_items = @{$overdues};
-    print <<"EOM";
-Fines assessment -- $today
+    if ($verbose) {
+        my $overdue_items = @{$overdues};
+        print <<"EOM";
+    Fines assessment -- $today
 EOM
-    if ($filename) {
-        say "Saved to $filename";
-    }
-    print <<"EOM";
-Number of Overdue Items:
-     counted $overdue_items
-    reported $counted
+        if ($filename) {
+            say "Saved to $filename";
+        }
+        print <<"EOM";
+    Number of Overdue Items:
+        counted $overdue_items
+        reported $counted
 
 EOM
+    }
+    _flock($LockFH, LOCK_UN);
+} else {
+    print "Skipping fines assessment because flock failed on $lockfile: $!\n";
 }
 
 sub set_holiday {
@@ -201,3 +230,28 @@ sub get_filename {
     }
     return $name;
 }
+
+sub _create_lockfile { #returns undef on failure
+    my $dir= shift;
+    unless (-d $dir) {
+        eval { mkpath($dir, 0, oct(755)) };
+        return if $@;
+    }
+    return if !open my $fh, q{>}, $dir.'/'.LOCK_FILENAME;
+    return ( $fh, $dir.'/'.LOCK_FILENAME );
+}
+
+sub _flock {
+    my ($fh, $op)= @_;
+    return 1 if !$use_flock;
+
+    #check if flock is present; if not, you will have a fatal error
+    my $lock_acquired = eval { flock($fh, $op) };
+
+    # assuming that $fh and $op are fine(..), an undef $lock_acquired
+    # means no flock
+    $use_flock = defined($lock_acquired) ? 1 : 0;
+    print "Warning: flock could not be used!\n" if !$use_flock;
+    return 1 if !$use_flock;
+    return $lock_acquired;
+}
-- 
2.20.1