From db9a773aa5b515f9ad469407d3673240531f28ed Mon Sep 17 00:00:00 2001 From: Bill Erickson Date: Mon, 27 Aug 2012 10:38:48 -0400 Subject: [PATCH] Repair fine generator memory leak 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 Signed-off-by: Mike Rylander --- .../OpenILS/Application/Storage/Publisher/action.pm | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm index ed5c4adb37..16e698f7c3 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm @@ -1052,7 +1052,7 @@ sub generate_fines { #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; @@ -1238,13 +1238,15 @@ sub generate_fines { )->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; - throw $e if ($e =~ /IS NOT CONNECTED TO THE NETWORK/o); - }; + last if ($e =~ /IS NOT CONNECTED TO THE NETWORK/o); + } } } __PACKAGE__->register_method( -- 2.43.2