Tue Feb 11 12:35:18 2020 futuramedium [...] yandex.ru - Correspondence added
Well, I had some time today, what follows is maybe not a patch "as is", rather invitation for a review. Plus, I didn't investigate why there were no warnings before mentioned commit -- just looking at current code (though I suspect it is related to efforts to prevent memory leaks, circular refs, etc., therefore there's no way back to "before that commit").
Reduced to SSCCE, the test would be:
use strict;
use warnings;
use PDF::API2;
my $pdf = PDF::API2-> open( 'blank217.pdf' );
my $outlines = $pdf-> outlines;
$outlines-> outline-> dest( $pdf-> openpage( $_ ))
for 0 .. 100;
$pdf-> { pdf }-> release;
There are from 0 to quite a few warnings then, different from run to run, and only reason for that is "values" returns random order in line 115
https://metacpan.org/release/PDF-API2/source/lib/PDF/API2/Basic/PDF/Objind.pm#L115Hm-m, OK. The Outlines tree is quite inter-connected, with Prev/Next links for intermediate/leaf, First/Last for root/intermediate, and Parent for all but root. I suspect, because of that, with just 101 bookmarks we easily (but not always) hit the deep recursion limit, then.
Actually, many PDF structures are similarly inter-connected -- e.g. consider a pages tree. I think, though didn't try to confirm, that a relatively (maybe a bit more) complex pages tree can trigger the same warnings when "released", because of Parent member.
I think, with Outlines tree, we don't have to follow any of Parent/First/Last/Prev/Next links to manually "release" them. For simple reason, that leafs/intermediate nodes are members of ' children' arrays. In fact, matter-of-fact autovivification of these arrays (
https://metacpan.org/release/PDF-API2/source/lib/PDF/API2/Outline.pm#L131) is somewhat ...disturbing. OK, what I'm suggesting, is that said "Parent/First/Last/Prev/Next" values are weakened, and "weak" refs ARE NOT added to queue to be manually released.
For Outlines, they are released anyway because there are stored in ' children'. For Pages, any "Parent" is already weakened if I'm reading the code right. With patches applied, there are no warnings however many times I run the test. I hope there are no memory leaks then. For structures other than Outlines or Pages, if PDF::API2 ever deals with them, the solution would be then to weaken refs which are released through other links -- then we don't hit "deep recursion" for trees of any complexity. What do you think?
--- PDF/API2/Outline_old.pm Thu Feb 6 01:08:40 2020
+++ PDF/API2/Outline.pm Tue Feb 11 20:16:25 2020
@@ -32,36 +32,43 @@
$self->{'Prev'} = $prev if defined $prev;
$self->{' api'} = $api;
weaken $self->{' api'};
+ weaken $self-> {'Parent'} if defined $parent;
+ weaken $self-> {'Prev'} if defined $prev;
return $self;
}
sub parent {
my $self = shift();
$self->{'Parent'} = shift() if defined $_[0];
+ weaken $self-> {'Parent'};
return $self->{'Parent'};
}
sub prev {
my $self = shift();
$self->{'Prev'} = shift() if defined $_[0];
+ weaken $self-> {'Prev'};
return $self->{'Prev'};
}
sub next {
my $self = shift();
$self->{'Next'} = shift() if defined $_[0];
+ weaken $self-> {'Next'};
return $self->{'Next'};
}
sub first {
my $self = shift();
$self->{'First'} = $self->{' children'}->[0] if defined $self->{' children'} and defined $self->{' children'}->[0];
+ weaken $self-> {'First'};
return $self->{'First'};
}
sub last {
my $self = shift();
$self->{'Last'} = $self->{' children'}->[-1] if defined $self->{' children'} and defined $self->{' children'}->[-1];
+ weaken $self-> {'Last'};
return $self->{'Last'};
}
---------------------------------
--- PDF/API2/Basic/PDF/Objind_old.pm Thu Feb 6 01:08:40 2020
+++ PDF/API2/Basic/PDF/Objind.pm Tue Feb 11 20:17:34 2020
@@ -14,6 +14,7 @@
use strict;
use warnings;
+use Scalar::Util 'isweak';
our $VERSION = '2.037'; # VERSION
@@ -112,7 +113,7 @@
sub release {
my ($self) = @_;
- my @tofree = values %$self;
+ my @tofree = grep { !isweak $_ } values %$self;
%$self = ();
while (my $item = shift @tofree) {
Tue Feb 11 12:41:58 2020 futuramedium [...] yandex.ru - Correspondence added
Sorry, it looks I added "fixed in 2.037"; I tried to add "my line numbers are for 2.037".
