[open-ils-commits] [GIT] Evergreen ILS branch rel_2_4 updated. 11324fd3722dc7e55700bacf538dd2026feb7820
Evergreen Git
git at git.evergreen-ils.org
Tue May 14 09:06:20 EDT 2013
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Evergreen ILS".
The branch, rel_2_4 has been updated
via 11324fd3722dc7e55700bacf538dd2026feb7820 (commit)
via a8e31242be0f54620c269d5f2e50c62010c3bd37 (commit)
from 73adbb3e56f8e0cad2997ea09d8257e3307b9934 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 11324fd3722dc7e55700bacf538dd2026feb7820
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date: Mon May 6 14:15:18 2013 -0400
Serials: MFHD::get_compressed_holdings() can reach infinite loop
Even controlled serials holdings involve the internal creation of MFHD
fields, upon which caculations are performed for such purposes as the
display of holdings summaries in the OPAC.
There are too many ways that incorrect MFHD (or MFHD that our code just
can't yet handle) can lead our MFHD routines to crash. We can't address
all these possibilities in a single bug fix. But we can avoid this
infinite loop.
A subroutine within open-ils.serial (_summarize_contents()) relies on
MFHD::get_compressed_holdings(). When the latter went into an infinite
loop the result would be an open-il.serial drone process consuming CPU
time indefinitely and, depending on the data that provoked the loop,
potentially writing repeating messages to stderr indefinitely.
End users will still see the item receiving fail in these cases, and be
obliged to work around the issue as before until more robust
holdings summarization code can be written, but at least the overall
condition of the running Evergreen system won't be affected, and there
will be better information in the error logs.
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Signed-off-by: Remington Steed <rjs7 at calvin.edu>
Signed-off-by: Dan Wells <dbw2 at calvin.edu>
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
index 19b14aa..6425257 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
@@ -644,6 +644,9 @@ sub scoped_bib_holdings_summary {
my %statement_blob;
for my $type ( keys %type_blob ) {
my ($mfhd,$list) = _summarize_contents(new_editor(), $type_blob{$type});
+
+ return {} if $U->event_code($mfhd); # _summarize_contents() failed, bad data?
+
$statement_blob{$type} = $list;
}
@@ -1496,6 +1499,7 @@ sub _prepare_unit {
}
my ($mfhd, $formatted_parts) = _summarize_contents($e, $issuances);
+ return $mfhd if $U->event_code($mfhd);
# special case for single formatted_part (may have summarized version)
if (@$formatted_parts == 1) {
@@ -1527,6 +1531,7 @@ sub _prepare_summaries {
my ($e, $issuances, $sdist, $type) = @_;
my ($mfhd, $formatted_parts) = _summarize_contents($e, $issuances, $sdist);
+ return $mfhd if $U->event_code($mfhd);
my $search_method = "search_serial_${type}_summary";
my $summary = $e->$search_method([{"distribution" => $sdist->id}]);
@@ -1812,7 +1817,6 @@ sub _build_unit {
return $unit;
}
-
sub _summarize_contents {
my $editor = shift;
my $issuances = shift;
@@ -1891,11 +1895,24 @@ sub _summarize_contents {
my @formatted_parts;
my @scap_fields_ordered = $mfhd->field('85[345]');
+
foreach my $scap_field (@scap_fields_ordered) { #TODO: use generic MFHD "summarize" method, once available
- my @updated_holdings = $mfhd->get_compressed_holdings($scap_field);
- foreach my $holding (@updated_holdings) {
- push(@formatted_parts, $holding->format);
- }
+ my @updated_holdings;
+ eval {
+ @updated_holdings = $mfhd->get_compressed_holdings($scap_field);
+ };
+ if ($@) {
+ my $msg = "get_compressed_holdings(): $@ ; using sdist ID #" .
+ ($sdist ? $sdist->id : "<NONE>") . " and " .
+ scalar(@$issuances) . " issuances, of which one has ID #" .
+ $issuances->[0]->id;
+
+ $msg =~ s/\n//gm;
+ $logger->error($msg);
+ return new OpenILS::Event("BAD_PARAMS", note => $msg);
+ }
+
+ push @formatted_parts, map { $_->format } @updated_holdings;
}
return ($mfhd, \@formatted_parts);
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD.pm
index 2bf94d4..32ea066 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD.pm
@@ -404,11 +404,23 @@ sub get_compressed_holdings {
last;
} else {
push(@comp_holdings, $curr_holding);
+
+ my $loop_count = 0;
+ (my $runner_dump = $runner->as_formatted) =~ s/\n\s+/*/gm; # logging
+
while ($runner le $holding) {
- # Here is where we used to get stuck in an infinite loop
- # until the "Don't know how to deal with frequency" was
- # elevated from a carp to a croak.
+ # Infinite loops used to happen here. As written today,
+ # ->increment() cannot be guaranteed to eventually falsify
+ # the condition ($runner le $holding) in certain cases.
+
$runner->increment;
+
+ if (++$loop_count >= 10000) {
+ (my $holding_dump = $holding->as_formatted) =~ s/\n\s+/*/gm;
+
+ croak "\$runner<$runner_dump> didn't catch up with " .
+ "\$holding<$holding_dump> after 10000 increments";
+ }
}
$curr_holding = $holding->clone;
$seqno++;
commit a8e31242be0f54620c269d5f2e50c62010c3bd37
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date: Mon May 6 14:13:13 2013 -0400
Serials: Test method to identify holdings that lead to summarization bugs
This adds a simple test method that allows testing of problem holdings.
See the next commit.
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Signed-off-by: Remington Steed <rjs7 at calvin.edu>
Signed-off-by: Dan Wells <dbw2 at calvin.edu>
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
index 377069d..19b14aa 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm
@@ -3643,4 +3643,35 @@ sub clone_subscription {
return $result;
}
+__PACKAGE__->register_method(
+ "method" => "summary_test",
+ "api_name" => "open-ils.serial.summary_test",
+ "stream" => 1,
+ "api_level" => 1,
+ "argc" => 3
+);
+
+# This crummy little test method allows quicker reproduction of certain
+# failures (e.g. at item receive time) of the holdings summarization code.
+# Pass it an authtoken, an array of issuance IDs, and a single sdist ID
+sub summary_test {
+ my ($self, $conn, $authtoken, $iss_id_list, $sdist_id) = @_;
+
+ my $e = new_editor(authtoken => $authtoken, xact => 1);
+ return $e->die_event unless $e->checkauth;
+ return $e->die_event unless $e->allowed("RECEIVE_SERIAL");
+
+ my @issuances;
+ foreach my $id (@$iss_id_list) {
+ my $iss = $e->retrieve_serial_issuance($id) or return $e->die_event;
+ push @issuances, $iss;
+ }
+
+ my $dist = $e->retrieve_serial_distribution($sdist_id) or return $e->die_event;
+
+ $conn->respond(_summarize_contents($e, \@issuances, $dist));
+ $e->rollback;
+ return;
+}
+
1;
-----------------------------------------------------------------------
Summary of changes:
.../src/perlmods/lib/OpenILS/Application/Serial.pm | 58 ++++++++++++++++++--
Open-ILS/src/perlmods/lib/OpenILS/Utils/MFHD.pm | 18 +++++-
2 files changed, 68 insertions(+), 8 deletions(-)
hooks/post-receive
--
Evergreen ILS
More information about the open-ils-commits
mailing list