[freeside-commits] freeside/FS/FS cust_main.pm, 1.271.2.18, 1.271.2.19 cust_bill.pm, 1.163.2.7, 1.163.2.8

[Ivan,,,]

Update of /home/cvs/cvsroot/freeside/FS/FS
In directory wavetail:/tmp/cvs-serv6059/FS/FS

Modified Files:
      Tag: FREESIDE_1_7_BRANCH
	cust_main.pm cust_bill.pm 
Log Message:
fix race condition where ->apply_payments_and_credits could double-apply in rare cases

Index: cust_main.pm
===================================================================
RCS file: /home/cvs/cvsroot/freeside/FS/FS/cust_main.pm,v
retrieving revision 1.271.2.18
retrieving revision 1.271.2.19
diff -u -d -r1.271.2.18 -r1.271.2.19
--- cust_main.pm	11 Jul 2007 08:35:40 -0000	1.271.2.18
+++ cust_main.pm	13 Jul 2007 23:52:31 -0000	1.271.2.19
@@ -3385,15 +3385,37 @@
 In most cases, this new method should be used in place of sequential
 apply_payments and apply_credits methods.
 
+If there is an error, returns the error, otherwise returns false.
+
 =cut
 
 sub apply_payments_and_credits {
   my $self = shift;
 
+  local $SIG{HUP} = 'IGNORE';
+  local $SIG{INT} = 'IGNORE';
+  local $SIG{QUIT} = 'IGNORE';
+  local $SIG{TERM} = 'IGNORE';
+  local $SIG{TSTP} = 'IGNORE';
+  local $SIG{PIPE} = 'IGNORE';
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
+  $self->select_for_update; #mutex
+
   foreach my $cust_bill ( $self->open_cust_bill ) {
-    $cust_bill->apply_payments_and_credits;
+    my $error = $cust_bill->apply_payments_and_credits;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "Error applying: $error";
+    }
   }
 
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+  ''; #no error
+
 }
 
 =item apply_credits OPTION => VALUE ...
@@ -3404,13 +3426,31 @@
 value of any remaining unapplied credits available for refund (see
 L<FS::cust_refund>).
 
+Dies if there is an error.
+
 =cut
 
 sub apply_credits {
   my $self = shift;
   my %opt = @_;
 
-  return 0 unless $self->total_credited;
+  local $SIG{HUP} = 'IGNORE';
+  local $SIG{INT} = 'IGNORE';
+  local $SIG{QUIT} = 'IGNORE';
+  local $SIG{TERM} = 'IGNORE';
+  local $SIG{TSTP} = 'IGNORE';
+  local $SIG{PIPE} = 'IGNORE';
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
+  $self->select_for_update; #mutex
+
+  unless ( $self->total_credited ) {
+    $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+    return 0;
+  }
 
   my @credits = sort { $b->_date <=> $a->_date} (grep { $_->credited > 0 }
       qsearch('cust_credit', { 'custnum' => $self->custnum } ) );
@@ -3439,13 +3479,20 @@
       'amount'  => $amount,
     } );
     my $error = $cust_credit_bill->insert;
-    die $error if $error;
+    if ( $error ) {
+      $dbh->rollback or die $dbh->errstr if $oldAutoCommit;
+      die $error;
+    }
     
     redo if ($cust_bill->owed > 0);
 
   }
 
-  return $self->total_credited;
+  my $total_credited = $self->total_credited;
+
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+
+  return $total_credited;
 }
 
 =item apply_payments
@@ -3455,11 +3502,26 @@
 
  #and returns the value of any remaining unapplied payments.
 
+Dies if there is an error.
+
 =cut
 
 sub apply_payments {
   my $self = shift;
 
+  local $SIG{HUP} = 'IGNORE';
+  local $SIG{INT} = 'IGNORE';
+  local $SIG{QUIT} = 'IGNORE';
+  local $SIG{TERM} = 'IGNORE';
+  local $SIG{TSTP} = 'IGNORE';
+  local $SIG{PIPE} = 'IGNORE';
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
+  $self->select_for_update; #mutex
+
   #return 0 unless
 
   my @payments = sort { $b->_date <=> $a->_date } ( grep { $_->unapplied > 0 }
@@ -3489,13 +3551,20 @@
       'amount' => $amount,
     } );
     my $error = $cust_bill_pay->insert;
-    die $error if $error;
+    if ( $error ) {
+      $dbh->rollback or die $dbh->errstr if $oldAutoCommit;
+      die $error;
+    }
 
     redo if ( $cust_bill->owed > 0);
 
   }
 
-  return $self->total_unapplied_payments;
+  my $total_unapplied_payments = $self->total_unapplied_payments;
+
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+
+  return $total_unapplied_payments;
 }
 
 =item total_credited
@@ -4837,8 +4906,12 @@
         return "can't bill customer for $line: $error";
       }
   
-      $cust_main->apply_payments_and_credits;
-  
+      $error = $cust_main->apply_payments_and_credits;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "can't bill customer for $line: $error";
+      }
+
       $error = $cust_main->collect();
       if ( $error ) {
         $dbh->rollback if $oldAutoCommit;

Index: cust_bill.pm
===================================================================
RCS file: /home/cvs/cvsroot/freeside/FS/FS/cust_bill.pm,v
retrieving revision 1.163.2.7
retrieving revision 1.163.2.8
diff -u -d -r1.163.2.7 -r1.163.2.8
--- cust_bill.pm	21 Jun 2007 04:02:40 -0000	1.163.2.7
+++ cust_bill.pm	13 Jul 2007 23:52:31 -0000	1.163.2.8
@@ -418,6 +418,19 @@
 sub apply_payments_and_credits {
   my $self = shift;
 
+  local $SIG{HUP} = 'IGNORE';
+  local $SIG{INT} = 'IGNORE';
+  local $SIG{QUIT} = 'IGNORE';
+  local $SIG{TERM} = 'IGNORE';
+  local $SIG{TSTP} = 'IGNORE';
+  local $SIG{PIPE} = 'IGNORE';
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
+  $self->select_for_update; #mutex
+
   my @payments = grep { $_->unapplied > 0 } $self->cust_main->cust_pay;
   my @credits  = grep { $_->credited > 0 } $self->cust_main->cust_credit;
 
@@ -483,10 +496,17 @@
     $app->invnum( $self->invnum );
 
     my $error = $app->insert;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "Error inserting ". $app->table. " record: $error";
+    }
     die $error if $error;
 
   }
 
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+  ''; #no error
+
 }
 
 =item generate_email PARAMHASH