Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in Plugin::Filter [rt.cpan.org #62045] #152

Closed
atoomic opened this issue Oct 5, 2018 · 3 comments
Closed

Memory leak in Plugin::Filter [rt.cpan.org #62045] #152

atoomic opened this issue Oct 5, 2018 · 3 comments

Comments

@atoomic
Copy link
Collaborator

atoomic commented Oct 5, 2018

Migrated from rt.cpan.org#62045 (status was 'open')

Requestors:

Attachments:

From hiratara@cpan.org on 2010-10-11 08:42:41:

Hi. I found a memory leak and reported. I used following simple script
to find the problem.

use strict;
use warnings;
use Template;

for my $l (1 .. 1000) {
	my $tpl = Template->new(PLUGIN_BASE  => 'test');
	$tpl->context->plugin('simple', []);

	warn `ps -o rss= -p $$` if $l % 10 == 0;
}

I read https://rt.cpan.org/Public/Bug/Display.html?id=46691 and
understood my patch had been reverted.

So I wrote another one. How is this patch? All tests in the t/ directory
are passed and there are no memory leaks.

Since I thought only the context should have the filter's reference, I
weakened another references.

From pepl@cpan.org on 2011-12-05 10:45:57:

I also experience leaks with that patch. Only the original one ('weaken
$this') works for me.
@atoomic
Copy link
Collaborator Author

atoomic commented Oct 5, 2018

I can reproduce this memory leak using a slightly modified version of yours
but Template::Plugin::Simple is not shipped

the leak is coming from $self->install_filter($name) called multiple times

#!/usr/bin/env perl

print $$ . "\n";

use strict;
use warnings;
use Template;
use Template::Plugin::Simple;

my $c = 0;
while (1) {
	++$c;
	
	my $tpl = Template->new(PLUGIN_BASE  => 'test');
	$tpl->context->plugin('Simple', []);

	
	if ( ($c % 10) == 0 ) {
		print qx{grep -i VMRSS /proc/$$/status} ."\n";	
		$c = 0;
	}
	
}

1;
__END__

@atoomic atoomic added the Bug label Oct 5, 2018
@atoomic atoomic self-assigned this Oct 9, 2018
@atoomic
Copy link
Collaborator Author

atoomic commented Oct 9, 2018

was able to detect the circular reference using Devel::Cycle

use Template;
use Template::Plugin::Simple;

use Devel::Cycle;

my $tpl = Template->new(PLUGIN_BASE  => 'test');
$tpl->context->plugin('Simple', []);

find_cycle( $tpl );
Cycle (1):
	     $Template::A->{'SERVICE'} => \%Template::Service::B
	$Template::Service::B->{'CONTEXT'} => \%Template::Context::C
	$Template::Context::C->{'LOAD_FILTERS'} => \@D
	                       $D->[0] => \%Template::Filters::E
	$Template::Filters::E->{'FILTERS'} => \%F
	                $F->{'simple'} => \@G
	                       $G->[0] => \&H
	             $H variable $this => \$I
	                           $$I => \%Template::Plugin::Simple::J
	$Template::Plugin::Simple::J->{'_CONTEXT'} => \%Template::Context::C

The leak is coming from Template::Plugin::Filter itself when setting the _CONTEXT...

A simple fix would be

diff --git a/lib/Template/Plugin/Filter.pm b/lib/Template/Plugin/Filter.pm
index 186c7738..c7b245a7 100644
--- a/lib/Template/Plugin/Filter.pm
+++ b/lib/Template/Plugin/Filter.pm
@@ -49,6 +49,9 @@ sub new {
         _CONFIG  => $config,
     }, $class;

+
+    weaken( $self->{_CONTEXT} );
+
     return $self->init($config)
         || $class->error($self->error());
 }

atoomic added a commit to atoomic/Template2 that referenced this issue Oct 9, 2018
GH abw#152

This is an issue reported by hiratara who detected
a memory leak in Template::Plugin::Filter.

This commit adds one extra dev test to check for memory
leak and report circular reference found, in order
to detect similar issues in the future.

The leak is fixed by using weaken on the context object
stored in Plugin::Filter.

References: RT 62045
@toddr
Copy link
Collaborator

toddr commented Oct 10, 2018

This has been fixed with #201

@toddr toddr closed this as completed Oct 10, 2018
atoomic added a commit to atoomic/Template2 that referenced this issue Oct 11, 2018
Fixup on GH abw#152

This is adding a test from GH abw#144 showing how
an incorrect weaken can break plugins.

Then it's patching correctly the circular reference when
building the filter using the factory.
atoomic added a commit to atoomic/Template2 that referenced this issue Oct 11, 2018
Fixup on GH abw#152

This is adding a test from GH abw#144 showing how
an incorrect weaken can break plugins.

Then it's patching correctly the circular reference when
building the filter using the factory.
atoomic added a commit to atoomic/Template2 that referenced this issue Oct 11, 2018
Fixup on GH abw#152

This is adding a test from GH abw#144 showing how
an incorrect weaken can break plugins.

Then it's patching correctly the circular reference when
building the filter using the factory.
atoomic added a commit to atoomic/Template2 that referenced this issue Oct 11, 2018
Fixup on GH abw#152

This is adding a test from GH abw#144 showing how
an incorrect weaken can break plugins.

Then it's patching correctly the circular reference when
building the filter using the factory.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Nov 24, 2018
Upstream changes:
Version 2.28 - 11th October 2018
#------------------------------------------------------------------------

* Add and enable Travis CI to track GitHub Pull Requests

* Template is now using GitHub as the official Bug Tracker

* Nicolas R. fixed a circular reference in Template::Plugin::Filter
  abw/Template2#152

* Nicolas R. adjusted group regexes to not be greedy
  abw/Template2#94

* Nicolas R. added unit tests to cover regression from RT 91172
  abw/Template2#122

* Nicolas R. added support for template files having mtime=0
  abw/Template2#102

* Todd Rinaldo fixed rand calls with no args in Math plugin
  abw/Template2#155

* Todd Rinaldo corrected ttree 2.22 logic change
  abw/Template2#148

* Todd Rinaldo turned off automated testing for tests using optional modules
  abw/Template2#156

* Nicolas R. adjusted unit tests to not force Stash::XS

* Nicolas R. added a pre allocated buffer in Stash.xs to avoid malloc/free
  abw/Template2#82

* Nicolas R. optmized Template::Parser by avoiding a dummy sub
  abw/Template2#83

* Nicolas R. optimized Template:Directive by using index
  abw/Template2#84

* Nicolas R. adjust _dotop logic in Stash for perl 5.28 and earlier
  abw/Template2#81

* Todd Rinaldo documented VMethod method called 'item'
  abw/Template2#90

* Nicolas R. adjusted t/filter.t after recent switch to RFC3986
  abw/Template2#179

* Nicolas R. fixed warnings from t/cgi.t
  abw/Template2#178

* Ivan Krylov added STRICT option to ttree
  abw/Template2#81

* Kent Fredric fixed relative path handling in templates on Perl 5.26+
  abw/Template2#80

* Tom Delmas fixed some typo from documentation
  abw/Template2#76

* Matthew Somerville switched uri/url to use RFC3986
  updated the documentation to match the history.
  abw/Template2#35

* Sebastien Deseille used remove_tree helper to remove directories
  abw/Template2#67

* Nick Hibma - Add Sortkeys to DUMPER_ARGS
  abw/Template2#64

* E. Choroba added a warn on duplicate block name
  abw/Template2#61

* Jason Lewis fixed some typo in ttree.pod
  abw/Template2#58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants