Repair fine generator memory leak
authorBill Erickson <berick@esilibrary.com>
Mon, 27 Aug 2012 14:38:48 +0000 (10:38 -0400)
committerMike Rylander <mrylander@gmail.com>
Thu, 13 Jun 2013 13:57:45 +0000 (09:57 -0400)
Calling "next" from within a "try" block results in a memory leak,
presumably because "try" is a tangled nest of subs and evals.
Replacing the "try" with a good ol' "eval" avoids the leak.

This can be reproduced with the following:

---------
use Error qw/:try/;

foreach (0..200000) {
    try {
        next;
    } catch Error with {
    };
}
---------

This particular leak in the fine generator is onerous when the fine
generator is run often (e.g. every 15 mins), which means circs that
have already been processed for the day are re-analzyed over and over,
causing the code to continue early (next) to the next loop iteration
for large numbers of circs.  It also happens when a circs are skipped
because they have no fine interval, rate, or max fine.

You know this is happening because you will see something like this in
the storage stderr log:

Exiting eval via next at
/usr/local/share/perl/5.10.1/OpenILS/Application/Storage/Publisher/action.pm
line 820.

Exiting subroutine via next at
/usr/local/share/perl/5.10.1/OpenILS/Application/Storage/Publisher/action.pm
line 820.

This patch does not avoid the "exiting eval via next" warning, since
we're still next'ing out of the eval.  It just avoids the memory leak
(and the "Exiting subroutine" warning).  More extensive refactoring is
needed to to completely remove the second warning.

Signed-off-by: Bill Erickson <berick@esilibrary.com>
Signed-off-by: Mike Rylander <mrylander@gmail.com>
Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm

index ed5c4ad..16e698f 100644 (file)
@@ -1052,7 +1052,7 @@ sub generate_fines {
         #TODO: reservation grace periods
         my $grace_period = ($is_reservation ? 0 : interval_to_seconds($c->grace_period));
 
         #TODO: reservation grace periods
         my $grace_period = ($is_reservation ? 0 : interval_to_seconds($c->grace_period));
 
-               try {
+               eval {
                        if ($self->method_lookup('open-ils.storage.transaction.current')->run) {
                                $log->debug("Cleaning up after previous transaction\n");
                                $self->method_lookup('open-ils.storage.transaction.rollback')->run;
                        if ($self->method_lookup('open-ils.storage.transaction.current')->run) {
                                $log->debug("Cleaning up after previous transaction\n");
                                $self->method_lookup('open-ils.storage.transaction.rollback')->run;
@@ -1238,13 +1238,15 @@ sub generate_fines {
                            )->gather(1);
                        }
 
                            )->gather(1);
                        }
 
-               } catch Error with {
-                       my $e = shift;
+               };
+
+               if ($@) {
+                       my $e = $@;
                        $client->respond( "Error processing overdue $ctype [".$c->id."]:\n\n$e\n" );
                        $log->error("Error processing overdue $ctype [".$c->id."]:\n$e\n");
                        $self->method_lookup('open-ils.storage.transaction.rollback')->run;
                        $client->respond( "Error processing overdue $ctype [".$c->id."]:\n\n$e\n" );
                        $log->error("Error processing overdue $ctype [".$c->id."]:\n$e\n");
                        $self->method_lookup('open-ils.storage.transaction.rollback')->run;
-                       throw $e if ($e =~ /IS NOT CONNECTED TO THE NETWORK/o);
-               };
+                       last if ($e =~ /IS NOT CONNECTED TO THE NETWORK/o);
+               }
        }
 }
 __PACKAGE__->register_method(
        }
 }
 __PACKAGE__->register_method(