[Rt-commit] rt branch 5.0/leaking-recursive-anon-subs created. rt-5.0.4-7-g6e064a65f9
BPS Git Server
git at git.bestpractical.com
Thu May 18 12:40:57 UTC 2023
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 "rt".
The branch, 5.0/leaking-recursive-anon-subs has been created
at 6e064a65f9bffa5b05929460292ab483209599f7 (commit)
- Log -----------------------------------------------------------------
commit 6e064a65f9bffa5b05929460292ab483209599f7
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date: Thu May 18 12:09:56 2023 +0300
Fix memory leaks in recursive anon subroutines
Introduce RT::Util::RecursiveSub that helps avoid memory leaks
in recursive anon subroutines.
diff --git a/lib/RT/Crypt.pm b/lib/RT/Crypt.pm
index 88ac450c0f..15f02bf440 100644
--- a/lib/RT/Crypt.pm
+++ b/lib/RT/Crypt.pm
@@ -52,6 +52,8 @@ use warnings;
package RT::Crypt;
use 5.010;
+use RT::Util ();
+
=head1 NAME
RT::Crypt - encrypt/decrypt and sign/verify subsystem for RT
@@ -393,14 +395,15 @@ sub FindProtectedParts {
if ( $args{'Scattered'} ) {
my %parent;
- my $filter; $filter = sub {
+ my $filter = RT::Util::RecursiveSub(sub {
+ my $self_cb = shift;
$parent{$_[0]} = $_[1];
unless ( $_[0]->is_multipart ) {
return () if $args{'Skip'}{$_[0]};
return $_[0];
}
- return map $filter->($_, $_[0]), grep !$args{'Skip'}{$_}, $_[0]->parts;
- };
+ return map $self_cb->($_, $_[0]), grep !$args{'Skip'}{$_}, $_[0]->parts;
+ });
my @parts = $filter->($entity);
return @res unless @parts;
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 444eb1c24e..0995a9e12a 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -72,6 +72,7 @@ use RT::Interface::Web::Menu;
use RT::Interface::Web::Session;
use RT::Interface::Web::Scrubber;
use RT::Interface::Web::Scrubber::Permissive;
+use RT::Util ();
use Digest::MD5 ();
use List::MoreUtils qw();
use JSON qw();
diff --git a/lib/RT/Util.pm b/lib/RT/Util.pm
index 16753a5b33..c3175230c9 100644
--- a/lib/RT/Util.pm
+++ b/lib/RT/Util.pm
@@ -54,6 +54,7 @@ use warnings;
use base 'Exporter';
our @EXPORT = qw/safe_run_child mime_recommended_filename EntityLooksLikeEmailMessage EmailContentTypes/;
+use Scalar::Util qw/weaken/;
use Encode qw/encode/;
sub safe_run_child (&) {
@@ -250,6 +251,40 @@ sub EmailContentTypes {
return ( 'message/rfc822', 'message/partial', 'message/external-body' );
}
+
+=head2 RecursiveSub
+
+When you need an anonymous sub that call itself, you can't just use:
+
+ my $sub;
+ $sub = sub {
+ my @args = @_;
+ ...
+ $sub->(@args);
+ ...
+ };
+
+as that will create a circular reference and leak memory. Instead, you should:
+
+ my $sub = RecursiveSub(sub {
+ my $self_cb = shift;
+ my @args = @_;
+ ...
+ $self_cb->(@args);
+ ,,,
+ });
+
+=cut
+
+sub RecursiveSub {
+ my $code = shift;
+ my ($sub, $wsub);
+ $sub = $wsub = sub { $code->($wsub, @_) };
+ weaken($wsub);
+ return $sub;
+}
+
+require RT::Base;
RT::Base->_ImportOverlays();
1;
diff --git a/share/html/Articles/Topics.html b/share/html/Articles/Topics.html
index 3f3def46cf..5fe697bb96 100644
--- a/share/html/Articles/Topics.html
+++ b/share/html/Articles/Topics.html
@@ -142,8 +142,8 @@ if ($id) {
$currtopic->Load($id);
}
-my $ProduceTree;
-$ProduceTree = sub {
+my $ProduceTree = RT::Util::RecursiveSub(sub {
+ my $self_cb = shift;
my ( $parentid ) = @_;
my $topics = RT::Topics->new( $session{'CurrentUser'} );
$topics->LimitToObject($currclass);
@@ -157,11 +157,11 @@ $ProduceTree = sub {
Topic => $t,
Class => $currclass_id,
);
- $ProduceTree->( $t->Id ) if $t->Children->Count;
+ $self_cb->( $t->Id ) if $t->Children->Count;
$m->out("</li>");
}
$m->out("</ul>");
-};
+});
my @articles;
if ($id) {
diff --git a/share/html/Elements/MyRT b/share/html/Elements/MyRT
index cdefb4d565..4cc8eb92ba 100644
--- a/share/html/Elements/MyRT
+++ b/share/html/Elements/MyRT
@@ -108,8 +108,8 @@ $sidebar = undef unless $sidebar && @$sidebar;
my $Rows = $user->Preferences( 'SummaryRows', ( RT->Config->Get('DefaultSummaryRows') || 10 ) );
-my $show_cb;
-$show_cb = sub {
+my $show_cb = RT::Util::RecursiveSub(sub {
+ my $self_cb = shift;
my $entry = shift;
my $type;
my $name;
@@ -169,13 +169,13 @@ $show_cb = sub {
}
my @panes = @{ $current_dashboard->Panes->{$entry->{pane}} || [] };
for my $portlet (@panes) {
- $show_cb->($portlet, $depth + 1);
+ $self_cb->($portlet, $depth + 1);
}
} else {
$RT::Logger->error("unknown portlet type '$type'");
}
}
-};
+});
</%INIT>
<%ARGS>
diff --git a/share/html/Elements/RT__Asset/ColumnMap b/share/html/Elements/RT__Asset/ColumnMap
index b6a50b7281..578ef038ec 100644
--- a/share/html/Elements/RT__Asset/ColumnMap
+++ b/share/html/Elements/RT__Asset/ColumnMap
@@ -50,12 +50,12 @@ $Name => undef
$Attr => undef
</%ARGS>
<%ONCE>
-my $linkUsers;
-$linkUsers = sub {
+my $linkUsers = RT::Util::RecursiveSub(sub {
+ my $self_cb = shift;
my ($what, $more) = @_;
if ($what->isa("RT::Group")) {
# Link the users (non-recursively)
- my @ret = map {$linkUsers->($_->[1], $more), ", "}
+ my @ret = map {$self_cb->($_->[1], $more), ", "}
sort {$a->[0] cmp $b->[0]}
map {+[($_->EmailAddress||''), $_]}
@{ $what->UserMembersObj( Recursively => 0 )->ItemsArrayRef };
@@ -71,7 +71,7 @@ $linkUsers = sub {
push @ret, $more->($what) if $more;
return @ret;
}
-};
+});
my $COLUMN_MAP = {
Name => {
attribute => 'Name',
diff --git a/share/html/Search/Elements/ConditionRow b/share/html/Search/Elements/ConditionRow
index 255bb5d03b..b53828ca96 100644
--- a/share/html/Search/Elements/ConditionRow
+++ b/share/html/Search/Elements/ConditionRow
@@ -62,15 +62,15 @@ return unless $Condition && $Condition->{'Name'};
$m->callback( Condition => \$Condition );
return unless $Condition;
-my $handle_block;
-$handle_block = sub {
+my $handle_block = RT::Util::RecursiveSub(sub {
+ my $self_cb = shift;
my $box = shift;
return $box unless ref $box;
my $name = shift;
if ( UNIVERSAL::isa($box, 'ARRAY') ) {
my $res = '';
- $res .= $handle_block->( $_, $name ) foreach @$box;
+ $res .= $self_cb->( $_, $name ) foreach @$box;
return $res;
}
@@ -100,7 +100,7 @@ $handle_block = sub {
$res .= qq{</select>};
return $res;
}
-};
+});
</%INIT>
<%ARGS>
diff --git a/share/html/Ticket/ShowEmailRecord.html b/share/html/Ticket/ShowEmailRecord.html
index 9dfbc1bb33..ab29446aea 100644
--- a/share/html/Ticket/ShowEmailRecord.html
+++ b/share/html/Ticket/ShowEmailRecord.html
@@ -69,8 +69,8 @@ my $show_content = sub {
$m->out( '<a href="'. $href .'">'. loc('download') .'</a>' );
};
-my $show;
-$show = sub {
+my $show = RT::Util::RecursiveSub(sub {
+ my $self_cb = shift;
my $attach = shift;
$m->out('</div>'); # Close rt-header-container
$m->out('<div id="body">');
@@ -85,14 +85,14 @@ $show = sub {
if ( $attach->ContentType =~ m{^multipart/}i ) {
my $children = $attach->Children;
while ( my $child = $children->Next ) {
- $show->( $child );
+ $self_cb->( $child );
}
} else {
$show_content->( $attach );
}
$m->out('</div>') if $plain_text_mono;
$m->out('</div>');
-};
+});
# Set error for error message below. Abort doesn't display well
# because ShowEmailRecord doesn't use the standard RT menus
-----------------------------------------------------------------------
hooks/post-receive
--
rt
More information about the rt-commit
mailing list