Skip Menu |

This queue is for tickets about the Set-Object CPAN distribution.

Report information
The Basics
Id: 69967
Status: resolved
Priority: 0/
Queue: Set-Object

People
Owner: Nobody in particular
Requestors: mp [...] leasingborsen.dk
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 1.28
Fixed in: (no value)



I think I've found a memory leak in Set::Object::Weak. The attached test case demonstrates the situation that fails, at least in my setup. The output from that script on my computer is: # linux x86_64-linux # perl version 5.012004 # Moose-2.0202 # Set::Object-1.28 # Test::LeakTrace-0.13 ok 1 - Testing Set::Object for leaking (leaks 0 <= 0) not ok 2 - Testing Set::Object::Weak for leaking (leaks 2 <= 0) # Failed test 'Testing Set::Object::Weak for leaking (leaks 2 <= 0)' # at Set_Object.t line 41. # '2' # <= # '0' # leaked SCALAR(0xeccce0) from Set_Object.t line 39. # 38: $set->insert($obj); # 39: $set->remove($obj); # 40: } # SV = IV(0xecccd8) at 0xeccce0 # REFCNT = 1 # FLAGS = (IOK,pIOK) # IV = 0 # leaked ARRAY(0x1283740) from Set_Object.t line 38. # 37: my $obj = Foo->new; # 38: $set->insert($obj); # 39: $set->remove($obj); # SV = PVAV(0x14b2f30) at 0x1283740 # REFCNT = 2 # FLAGS = () # ARRAY = 0x14cbea0 # FILL = 0 # MAX = 3 # ARYLEN = 0x0 # FLAGS = (REAL) # Elt No. 0 # SV = IV(0xecccd8) at 0xeccce0 # REFCNT = 1 # FLAGS = (IOK,pIOK) # IV = 0 1..2 # Looks like you failed 1 test of 2. Everything looks fine on the surface; $obj is free, when it leaves the scope, and $set looks healthy when I inspect it e.g. with Data::Dump. However, something still leaks, so my guess is that it's somewhere in the XS.
Subject: Set_Object.t
use strict; use warnings; use Config; use Test::More; use Test::LeakTrace; use Set::Object; { package Foo; use Moose; 1; } { no strict; note join ' ', map {$Config{$_}} qw(osname archname); note 'perl version ', $]; note $_,'-',${"${_}::VERSION"} for qw{Moose Set::Object Test::LeakTrace}; } my $set; { $set = Set::Object->new; no_leaks_ok { { my $obj = Foo->new; $set->insert($obj); $set->remove($obj); } } 'Testing Set::Object for leaking'; } { $set = Set::Object::Weak->new; no_leaks_ok { { my $obj = Foo->new; $set->insert($obj); $set->remove($obj); } } 'Testing Set::Object::Weak for leaking'; } done_testing;
From: mp [...] leasingborsen.dk
...and just to demonstrate that it does infact leak, try running the above script and what the process' memory use. On my setup the memory usage grows from 15MB to 195MB from start to end of the script.
Subject: Set_Object_blackbox.pl
use strict; use warnings; use Set::Object; use Devel::Monitor qw(:all); use Test::LeakTrace; { package Foo; use Moose; 1; } my $set = Set::Object::Weak->new; for my $outer (1..100){ for my $inner (1..10000){ my $obj = Foo->new; $set->insert($obj); $set->remove($obj); } print "$outer\n"; }
Subject: Memory leak in Set::Object::Weak
From: mp [...] leasingborsen.dk
...and the subject should have been "Memory leak in Set::Object::Weak"
Subject: Patch for memory leak in Object.xs
From: mp [...] leasingborsen.dk
I think that I've found and plugged the leak now. My patch doesn't take older Perl versions into account.
Subject: Object.xs.patch
--- Set-Object-1.28/Object.xs 2010-07-14 06:42:11.000000000 +0200 +++ /home/parmus/Projects/Set-Object-1.28/Object.xs 2011-08-04 02:01:56.357000046 +0200 @@ -358,23 +358,8 @@ i--; } if (!c) { - /* we should clear the magic, really. */ - MAGIC* last = 0; - for (mg = SvMAGIC(sv); mg; mg = mg->mg_moremagic) { - if (mg->mg_type == SET_OBJECT_MAGIC_backref) { - if (last) { - last->mg_moremagic = mg->mg_moremagic; - Safefree(mg); - break; - } else if (mg->mg_moremagic) { - SvMAGIC(sv) = mg->mg_moremagic; - } else { - SvMAGIC(sv) = 0; - SvAMAGIC_off(sv); - } - } - last=mg; - } + sv_unmagic(sv, SET_OBJECT_MAGIC_backref); + SvREFCNT_dec(wand); } } }
Subject: New tests and tools
From: mp [...] leasingborsen.dk
...and here are the scripts I used to demonstrate and track down the leak.
Subject: Set-Object-1.28-tools.patch
diff -uNr '--exclude=.project' Set-Object-1.28-orig//t/misc/internal_leaks.t Set-Object-1.28/t/misc/internal_leaks.t --- Set-Object-1.28-orig//t/misc/internal_leaks.t 1970-01-01 01:00:00.000000000 +0100 +++ Set-Object-1.28/t/misc/internal_leaks.t 2011-08-04 11:35:48.750000048 +0200 @@ -0,0 +1,46 @@ +#!/usr/bin/perl -w +use strict; +use warnings; +use Config; +use Test::More; +use Test::LeakTrace; +use Set::Object; + +{ + package Foo; + use Moose; + __PACKAGE__->meta->make_immutable; + 1; +} + +{ + no strict; + note join ' ', map {$Config{$_}} qw(osname archname); + note 'perl version ', $]; + note $_,'-',${"${_}::VERSION"} for qw{Moose Set::Object Test::LeakTrace}; +} + +my $set; +{ + $set = Set::Object->new; + no_leaks_ok { + { + my $obj = Foo->new; + $set->insert($obj); + $set->remove($obj); + } + } 'Testing Set::Object for leaking'; +} + +{ + $set = Set::Object::Weak->new; + no_leaks_ok { + { + my $obj = Foo->new; + $set->insert($obj); + $set->remove($obj); + } + } 'Testing Set::Object::Weak for leaking'; +} + +done_testing; \ No newline at end of file diff -uNr '--exclude=.project' Set-Object-1.28-orig//tools/memory_usage.pl Set-Object-1.28/tools/memory_usage.pl --- Set-Object-1.28-orig//tools/memory_usage.pl 1970-01-01 01:00:00.000000000 +0100 +++ Set-Object-1.28/tools/memory_usage.pl 2011-08-04 11:35:44.062000049 +0200 @@ -0,0 +1,31 @@ +#!/usr/bin/perl -w +use strict; +use warnings; +use Set::Object; +use Memory::Usage; + +{ + package Foo; + use Moose; + __PACKAGE__->meta->make_immutable; + 1; +} + +my $mu = Memory::Usage->new; + +$mu->record('Start'); + +{ + my $set = Set::Object::Weak->new; + $mu->record('After creating the set'); + for my $outer (1..5){ + for my $inner (1..10000){ + my $obj = Foo->new; + $set->insert($obj); + $set->remove($obj); + } + $mu->record(join ' ', 'After doing', 10000*$outer, 'inserts and removes'); + } +} +$mu->record('After destroying the set'); +print $mu->report; \ No newline at end of file diff -uNr '--exclude=.project' Set-Object-1.28-orig//tools/valgrind_weak.pl Set-Object-1.28/tools/valgrind_weak.pl --- Set-Object-1.28-orig//tools/valgrind_weak.pl 1970-01-01 01:00:00.000000000 +0100 +++ Set-Object-1.28/tools/valgrind_weak.pl 2011-08-04 11:35:41.246000047 +0200 @@ -0,0 +1,25 @@ +#!/usr/bin/perl -w +use strict; +use warnings; +use Test::More; +use Set::Object; +use Scalar::Util qw(refaddr); + +{ + package Foo; + use Moose; + __PACKAGE__->meta->make_immutable; + 1; +} + +eval 'use Test::Valgrind'; +plan skip_all => 'Test::Valgrind is required to test your distribution with valgrind' if $@; + +{ + my $set = Set::Object::Weak->new; + { + my $obj = Foo->new; + $set->insert($obj); + $set->remove($obj); + } +} \ No newline at end of file
Thanks for the patch! Applied as https://github.com/samv/Set- Object/commit/25686d92bd42ce3b8a220fb0b7a74a97e04dbae8 - sorry I didn't have a name to credit, so I credited you by email address.