Skip to content

Commit

Permalink
resolve #1 - fix reference cycle in reggrp closures
Browse files Browse the repository at this point in the history
replace use of $self in the reggrp_data closures that was causing
the following memory cycle (amongst others):

    Cycle #1
        JavaScript::Packer A->{_reggrp_comments} => Regexp::RegGrp B
        Regexp::RegGrp B->{_reggrp} => @C
        @c->[4] => Regexp::RegGrp::Data D
        Regexp::RegGrp::Data D->{_replacement} => &E
        closure &E, $self => $F
        $F => JavaScript::Packer A

now we are using package variables set when minify is called thus
handle any change to the state and remove the cycles

bump VERSION and Changes for CPAN release
  • Loading branch information
leejo committed May 22, 2016
1 parent bd371b1 commit f2d336f
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 10 deletions.
3 changes: 3 additions & 0 deletions Changes
@@ -1,5 +1,8 @@
Revision history for JavaScript-Packer

2.01 2016-06-22
- Fix refrence cycles in ->init method causing memory leaks (GH #1)

2.00 2015-05-27
- New maintainer: LEEJO
- Merge PR from dod38fr (nevesenin/javascript-packer-perl:GH #8, nevesenin/javascript-packer-perl:GH #9)
Expand Down
2 changes: 1 addition & 1 deletion MANIFEST
Expand Up @@ -15,11 +15,11 @@ inc/Module/Install/WriteAll.pm
lib/JavaScript/Packer.pm
Makefile.PL
MANIFEST
META.yml
README
t/00-load.t
t/01-interface.t
t/02-io.t
t/03-leaks.t
t/pod.t
t/scripts/s1-expected.js
t/scripts/s1.js
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -10,7 +10,7 @@ JavaScript::Packer - Perl version of Dean Edwards' Packer.js

# VERSION

Version 2.00
Version 2.01

# DESCRIPTION

Expand Down
29 changes: 21 additions & 8 deletions lib/JavaScript/Packer.pm
Expand Up @@ -8,7 +8,7 @@ use Regexp::RegGrp;

# =========================================================================== #

our $VERSION = "2.00";
our $VERSION = "2.01";

our @BOOLEAN_ACCESSORS = ( 'no_compress_comment', 'remove_copyright' );

Expand Down Expand Up @@ -215,6 +215,14 @@ sub compress {
return $self->{_compress};
}

# these variables are used in the closures defined in the init function
# below - we have to use globals as using $self within the closures leads
# to a reference cycle and thus memory leak, and we can't scope them to
# the init method as they may change. they are set by the minify sub
our $reggrp_comments;
our $reggrp_clean;
our $reggrp_whitespace;

sub init {
my $class = shift;
my $self = {};
Expand Down Expand Up @@ -293,9 +301,9 @@ sub init {
$self->{comments}->{reggrp_data}->[-2]->{replacement} = sub {
my $submatches = $_[0]->{submatches};
if ( $submatches->[0] eq '@' ) {
$self->reggrp_comments()->exec( \$submatches->[1] );
$self->reggrp_clean()->exec( \$submatches->[1] );
$self->reggrp_whitespace()->exec( \$submatches->[1] );
$reggrp_comments->exec( \$submatches->[1] );
$reggrp_clean->exec( \$submatches->[1] );
$reggrp_whitespace->exec( \$submatches->[1] );

return sprintf( "//%s%s\n%s", @{$submatches}[0 .. 2] );
}
Expand All @@ -307,9 +315,9 @@ sub init {
if ( $submatches->[0] =~ /^\/\*\@(.*)\@\*\/$/sm ) {
my $cmnt = $1;

$self->reggrp_comments()->exec( \$cmnt );
$self->reggrp_clean()->exec( \$cmnt );
$self->reggrp_whitespace()->exec( \$cmnt );
$reggrp_comments->exec( \$cmnt );
$reggrp_clean->exec( \$cmnt );
$reggrp_whitespace->exec( \$cmnt );

return sprintf( '/*@%s@*/ %s', $cmnt, $submatches->[1] );
}
Expand Down Expand Up @@ -373,6 +381,11 @@ sub minify {
}
}

# (re)initialize variables used in the closures
$reggrp_comments = $self->reggrp_comments;
$reggrp_clean = $self->reggrp_clean;
$reggrp_whitespace = $self->reggrp_whitespace;

my $copyright_comment = '';

if ( ${$javascript} =~ /$COPYRIGHT_COMMENT/ism ) {
Expand Down Expand Up @@ -718,7 +731,7 @@ JavaScript::Packer - Perl version of Dean Edwards' Packer.js
=head1 VERSION
Version 2.00
Version 2.01
=head1 DESCRIPTION
Expand Down
35 changes: 35 additions & 0 deletions t/03-leaks.t
@@ -0,0 +1,35 @@
#!perl

use strict;
use warnings;

use Test::More;
use JavaScript::Packer;

if (! eval "use Test::Memory::Cycle; 1;" ) {
plan skip_all => 'Test::Memory::Cycle required for this test';
}

my $packer = JavaScript::Packer->init;
memory_cycle_ok( $packer );

my $js = '
$(document).ready(function(e) {
try {
// $("body select").msDropDown();
$("#payin").msDropdown({visibleRows:3,rowHeight:30});
$("#payout").msDropdown({visibleRows:8,rowHeight:30});
$("#lang").msDropdown({visibleRows:2,rowHeight:16});
} catch(e) {
alert(e.message);
}
});
';

for ( 1 .. 5 ) {
ok( $packer->minify( \$js,{} ),'minify' );
}

memory_cycle_ok( $packer );
done_testing();

0 comments on commit f2d336f

Please sign in to comment.