X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=t%2F52leaks.t;h=4923be01d2aa4358bb77c6da21fbfcccd7581876;hb=people%2Fabraxxa%2Fhas_relationship_loaded;hp=1a052ef80c02f671597bc112dcb625799c598f0e;hpb=cd1228209f685767668163c2510723959951641b;p=dbsrgits%2FDBIx-Class.git diff --git a/t/52leaks.t b/t/52leaks.t index 1a052ef..4923be0 100644 --- a/t/52leaks.t +++ b/t/52leaks.t @@ -23,26 +23,38 @@ use Test::More; my $TB = Test::More->builder; if ($ENV{DBICTEST_IN_PERSISTENT_ENV}) { - # without this explicit close ->reset below warns - close ($TB->$_) for qw/output failure_output/; + # without this explicit close older TBs warn in END after a ->reset + if ($TB->VERSION < 1.005) { + close ($TB->$_) for (qw/output failure_output todo_output/); + } - # so done_testing can work - $TB->reset; + # if I do not do this, I get happy sigpipes on new TB, no idea why + # (the above close-and-forget doesn't work - new TB does *not* reopen + # its handles automatically anymore) + else { + for (qw/failure_output todo_output/) { + close $TB->$_; + open ($TB->$_, '>&', *STDERR); + } - # this simulates a subtest - $TB->_indent(' ' x 4); + close $TB->output; + open ($TB->output, '>&', *STDOUT); + } + + # so done_testing can work on every persistent pass + $TB->reset; } use lib qw(t/lib); use DBICTest::RunMode; +use DBICTest::Util::LeakTracer qw/populate_weakregistry assert_empty_weakregistry/; +use Scalar::Util 'refaddr'; use DBIx::Class; BEGIN { plan skip_all => "Your perl version $] appears to leak like a sieve - skipping test" - if DBIx::Class::_ENV_::PEEPEENESS(); + if DBIx::Class::_ENV_::PEEPEENESS; } -use Scalar::Util qw/refaddr reftype weaken/; - # this is what holds all weakened refs to be checked for leakage my $weak_registry = {}; @@ -52,19 +64,6 @@ my $has_dt; # Skip the heavy-duty leak tracing when just doing an install unless (DBICTest::RunMode->is_plain) { - # have our own little stack maker - Carp infloops due to the bless override - my $trace = sub { - my $depth = 1; - my (@stack, @frame); - - while (@frame = caller($depth++)) { - push @stack, [@frame[3,1,2]]; - } - - $stack[0][0] = ''; - return join "\tinvoked as ", map { sprintf ("%s at %s line %d\n", @$_ ) } @stack; - }; - # redefine the bless override so that we can catch each and every object created no warnings qw/redefine once/; no strict qw/refs/; @@ -80,29 +79,24 @@ unless (DBICTest::RunMode->is_plain) { } ); - my $slot = (sprintf '%s=%s(0x%x)', # so we don't trigger stringification - ref $obj, - reftype $obj, - refaddr $obj, - ); + # unicode is tricky, and now we happen to invoke it early via a + # regex in connection() + return $obj if (ref $obj) =~ /^utf8/; - # weaken immediately to avoid weird side effects - $weak_registry->{$slot} = { weakref => $obj, strace => $trace->() }; - weaken $weak_registry->{$slot}{weakref}; + # Test Builder is now making a new object for every pass/fail (que bloat?) + # and as such we can't really store any of its objects (since it will + # re-populate the registry while checking it, ewwww!) + return $obj if (ref $obj) =~ /^TB2::/; - return $obj; + # weaken immediately to avoid weird side effects + return populate_weakregistry ($weak_registry, $obj ); }; require Try::Tiny; for my $func (qw/try catch finally/) { my $orig = \&{"Try::Tiny::$func"}; *{"Try::Tiny::$func"} = sub (&;@) { - - my $slot = sprintf ('CODE(0x%x)', refaddr $_[0]); - - $weak_registry->{$slot} = { weakref => $_[0], strace => $trace->() }; - weaken $weak_registry->{$slot}{weakref}; - + populate_weakregistry( $weak_registry, $_[0] ); goto $orig; } } @@ -117,6 +111,7 @@ unless (DBICTest::RunMode->is_plain) { require DBI; require DBD::SQLite; require FileHandle; + require Moo; %$weak_registry = (); } @@ -136,11 +131,16 @@ unless (DBICTest::RunMode->is_plain) { # txn_do to invoke more codepaths my ($mc_row_obj, $pager, $pager_explicit_count) = $schema->txn_do (sub { - my $artist = $rs->create ({ + my $artist = $schema->resultset('Artist')->create ({ name => 'foo artist', cds => [{ title => 'foo cd', year => 1984, + tracks => [ + { title => 't1' }, + { title => 't2' }, + ], + genre => { name => 'mauve' }, }], }); @@ -151,6 +151,32 @@ unless (DBICTest::RunMode->is_plain) { return ($artist, $pg, $pg_wcount); }); + # more codepaths - error handling in txn_do + { + eval { $schema->txn_do ( sub { + $storage->_dbh->begin_work; + fail ('how did we get so far?!'); + } ) }; + + eval { $schema->txn_do ( sub { + $schema->txn_do ( sub { + die "It's called EXCEPTION"; + fail ('how did we get so far?!'); + } ); + fail ('how did we get so far?!'); + } ) }; + like( $@, qr/It\'s called EXCEPTION/, 'Exception correctly propagated in nested txn_do' ); + } + + # dbh_do codepath + my ($rs_bind_circref, $cond_rowobj) = $schema->storage->dbh_do ( sub { + my $row = $_[0]->schema->resultset('Artist')->new({}); + my $rs = $_[0]->schema->resultset('Artist')->search({ + name => $row, # this is deliberately bogus, see FIXME below! + }); + return ($rs, $row); + }); + is ($pager->next_page, 3, 'There is one more page available'); # based on 66 per 10 pages @@ -175,28 +201,73 @@ unless (DBICTest::RunMode->is_plain) { $schema->txn_rollback; } + # prefetching + my $cds_rs = $schema->resultset('CD'); + my $cds_with_artist = $cds_rs->search({}, { prefetch => 'artist' }); + my $cds_with_tracks = $cds_rs->search({}, { prefetch => 'tracks' }); + my $cds_with_stuff = $cds_rs->search({}, { prefetch => [ 'genre', { artist => { cds => { tracks => 'cd_single' } } } ] }); + + # implicit pref + my $cds_with_impl_artist = $cds_rs->search({}, { columns => [qw/me.title artist.name/], join => 'artist' }); + + # get_column + my $getcol_rs = $cds_rs->get_column('me.cdid'); + my $pref_getcol_rs = $cds_with_stuff->get_column('me.cdid'); + + # fire the column getters + my @throwaway = $pref_getcol_rs->all; + my $base_collection = { resultset => $rs, + pref_precursor => $cds_rs, + + pref_rs_single => $cds_with_artist, + pref_rs_multi => $cds_with_tracks, + pref_rs_nested => $cds_with_stuff, + + pref_rs_implicit => $cds_with_impl_artist, + + pref_row_single => $cds_with_artist->next, + pref_row_multi => $cds_with_tracks->next, + pref_row_nested => $cds_with_stuff->next, + + # even though this does not leak Storable croaks on it :((( + #pref_row_implicit => $cds_with_impl_artist->next, + + get_column_rs_plain => $getcol_rs, + get_column_rs_pref => $pref_getcol_rs, + # twice so that we make sure only one H::M object spawned chained_resultset => $rs->search_rs ({}, { '+columns' => [ 'foo' ] } ), chained_resultset2 => $rs->search_rs ({}, { '+columns' => [ 'bar' ] } ), row_object => $row_obj, + mc_row_object => $mc_row_obj, + result_source => $rs->result_source, result_source_handle => $rs->result_source->handle, pager_explicit_count => $pager_explicit_count, + leaky_resultset => $rs_bind_circref, + leaky_resultset_cond => $cond_rowobj, }; + # this needs to fire, even if it can't find anything + # see FIXME below + # we run this only on smokers - trying to establish a pattern + $rs_bind_circref->next + if ( ($ENV{TRAVIS}||'') ne 'true' and DBICTest::RunMode->is_smoker); + require Storable; %$base_collection = ( %$base_collection, refrozen => Storable::dclone( $base_collection ), rerefrozen => Storable::dclone( Storable::dclone( $base_collection ) ), + pref_row_implicit => $cds_with_impl_artist->next, schema => $schema, storage => $storage, sql_maker => $storage->sql_maker, @@ -225,14 +296,25 @@ unless (DBICTest::RunMode->is_plain) { $base_collection->{"DBI handle $_"} = $_; } - if ( DBIx::Class::Optional::Dependencies->req_ok_for ('test_leaks') ) { - Test::Memory::Cycle::memory_cycle_ok ($base_collection, 'No cycles in the object collection') - } + SKIP: { + if ( DBIx::Class::Optional::Dependencies->req_ok_for ('test_leaks') ) { + my @w; + local $SIG{__WARN__} = sub { $_[0] =~ /\QUnhandled type: REGEXP/ ? push @w, @_ : warn @_ }; - for (keys %$base_collection) { - $weak_registry->{"basic $_"} = { weakref => $base_collection->{$_} }; - weaken $weak_registry->{"basic $_"}{weakref}; + Test::Memory::Cycle::memory_cycle_ok ($base_collection, 'No cycles in the object collection'); + + if ( $] > 5.011 ) { + local $TODO = 'Silence warning due to RT56681'; + is (@w, 0, 'No Devel::Cycle emitted warnings'); + } + } + else { + skip 'Circular ref test needs ' . DBIx::Class::Optional::Dependencies->req_missing_for ('test_leaks'), 1; + } } + + populate_weakregistry ($weak_registry, $base_collection->{$_}, "basic $_") + for keys %$base_collection; } # check that "phantom-chaining" works - we never lose track of the original $schema @@ -240,7 +322,7 @@ unless (DBICTest::RunMode->is_plain) { { my $phantom; for ( - sub { DBICTest->init_schema }, + sub { DBICTest->init_schema( sqlite_use_file => 0 ) }, sub { shift->source('Artist') }, sub { shift->resultset }, sub { shift->result_source }, @@ -250,8 +332,12 @@ unless (DBICTest::RunMode->is_plain) { sub { shift->result_source }, sub { shift->schema }, sub { shift->clone }, - sub { shift->resultset('Artist') }, + sub { shift->resultset('CD') }, + sub { shift->next }, + sub { shift->artist }, + sub { shift->search_related('cds') }, sub { shift->next }, + sub { shift->search_related('artist') }, sub { shift->result_source }, sub { shift->resultset }, sub { shift->create({ name => 'detached' }) }, @@ -260,15 +346,7 @@ unless (DBICTest::RunMode->is_plain) { sub { shift->delete }, sub { shift->insert }, ) { - $phantom = $_->($phantom); - - my $slot = (sprintf 'phantom %s=%s(0x%x)', # so we don't trigger stringification - ref $phantom, - reftype $phantom, - refaddr $phantom, - ); - $weak_registry->{$slot} = $phantom; - weaken $weak_registry->{$slot}; + $phantom = populate_weakregistry ( $weak_registry, scalar $_->($phantom) ); } ok( $phantom->in_storage, 'Properly deleted/reinserted' ); @@ -282,131 +360,124 @@ for my $slot (keys %$weak_registry) { # T::B 2.0 has result objects and other fancyness delete $weak_registry->{$slot}; } - elsif ($slot =~ /^SQL::Translator/) { - # SQLT is a piece of shit, leaks all over + elsif ($slot =~ /^Method::Generate::(?:Accessor|Constructor)/) { + # Moo keeps globals around, this is normal delete $weak_registry->{$slot}; } + elsif ($slot =~ /^SQL::Translator::Generator::DDL::SQLite/) { + # SQLT::Producer::SQLite keeps global generators around for quoted + # and non-quoted DDL, allow one for each quoting style + delete $weak_registry->{$slot} + unless $cleared->{sqlt_ddl_sqlite}->{@{$weak_registry->{$slot}{weakref}->quote_chars}}++; + } elsif ($slot =~ /^Hash::Merge/) { # only clear one object of a specific behavior - more would indicate trouble delete $weak_registry->{$slot} unless $cleared->{hash_merge_singleton}{$weak_registry->{$slot}{weakref}{behavior}}++; } - elsif (DBIx::Class::_ENV_::INVISIBLE_DOLLAR_AT and $slot =~ /^__TxnScopeGuard__FIXUP__/) { - delete $weak_registry->{$slot} - } elsif ($slot =~ /^DateTime::TimeZone/) { # DT is going through a refactor it seems - let it leak zones for now delete $weak_registry->{$slot}; } } - -# FIXME -# For reasons I can not yet fully understand the table() god-method (located in -# ::ResultSourceProxy::Table) attaches an actual source instance to each class -# as virtually *immortal* class-data. -# For now just ignore these instances manually but there got to be a saner way -for ( map { $_->result_source_instance } ( - 'DBICTest::BaseResult', - map { DBICTest::Schema->class ($_) } DBICTest::Schema->sources -)) { - delete $weak_registry->{$_}; -} - -# FIXME -# same problem goes for the schema - its classdata contains live result source -# objects, which to add insult to the injury are *different* instances from the -# ones we ignored above -for ( values %{DBICTest::Schema->source_registrations || {}} ) { - delete $weak_registry->{$_}; -} - -for my $slot (sort keys %$weak_registry) { - - ok (! defined $weak_registry->{$slot}{weakref}, "No leaks of $slot") or do { - my $diag = ''; - - $diag .= Devel::FindRef::track ($weak_registry->{$slot}{weakref}, 20) . "\n" - if ( $ENV{TEST_VERBOSE} && eval { require Devel::FindRef }); - - if (my $stack = $weak_registry->{$slot}{strace}) { - $diag .= " Reference first seen$stack"; - } - - diag $diag if $diag; - }; +# FIXME !!! +# There is an actual strong circular reference taking place here, but because +# half of it is in XS no leaktracer sees it, and Devel::FindRef is equally +# stumped when trying to trace the origin. The problem is: +# +# $cond_object --> result_source --> schema --> storage --> $dbh --> {CachedKids} +# ^ / +# \-------- bound value on prepared/cached STH <-----------/ +# +{ + local $TODO = 'This fails intermittently - see RT#82942'; + if ( my $r = $weak_registry->{'basic leaky_resultset_cond'}{weakref} ) { + ok(! defined $r, 'Self-referential RS conditions no longer leak!') + or $r->result_source(undef); + } } +assert_empty_weakregistry ($weak_registry); # we got so far without a failure - this is a good thing # now let's try to rerun this script under a "persistent" environment # this is ugly and dirty but we do not yet have a Test::Embedded or # similar -my @pperl_cmd = (qw/pperl --prefork=1/, __FILE__); -my @pperl_term_cmd = @pperl_cmd; -splice @pperl_term_cmd, 1, 0, '--kill'; +# set up -I +require Config; +$ENV{PERL5LIB} = join ($Config::Config{path_sep}, @INC); +($ENV{PATH}) = $ENV{PATH} =~ /(.+)/; + + +my $persistence_tests = { + PPerl => { + cmd => [qw/pperl --prefork=1/, __FILE__], + }, + 'CGI::SpeedyCGI' => { + cmd => [qw/speedy -- -t5/, __FILE__], + }, +}; # scgi is smart and will auto-reap after -t amount of seconds -my @scgi_cmd = (qw/speedy -- -t5/, __FILE__); +# pperl needs an actual killer :( +$persistence_tests->{PPerl}{termcmd} = [ + $persistence_tests->{PPerl}{cmd}[0], + '--kill', + @{$persistence_tests->{PPerl}{cmd}}[ 1 .. $#{$persistence_tests->{PPerl}{cmd}} ], +]; SKIP: { skip 'Test already in a persistent loop', 1 if $ENV{DBICTEST_IN_PERSISTENT_ENV}; - skip 'Persistence test disabled on regular installs', 1 - if DBICTest::RunMode->is_plain; - skip 'Main test failed - skipping persistent env tests', 1 unless $TB->is_passing; - # set up -I - require Config; - local $ENV{PERL5LIB} = join ($Config::Config{path_sep}, @INC); - local $ENV{DBICTEST_IN_PERSISTENT_ENV} = 1; - # try with pperl - SKIP: { - skip 'PPerl persistent environment tests require PPerl', 1 - unless eval { require PPerl }; + require IPC::Open2; - # since PPerl is racy and sucks - just prime the "server" - { - local $ENV{DBICTEST_PERSISTENT_ENV_BAIL_EARLY} = 1; - system(@pperl_cmd); - sleep 1; - - # see if it actually runs - if not might as well bail now - skip "Something is wrong with pperl ($!)", 1 - if system(@pperl_cmd); + for my $type (keys %$persistence_tests) { SKIP: { + unless (eval "require $type") { + # Don't terminate what we didn't start + delete $persistence_tests->{$type}{termcmd}; + skip "$type module not found", 1; } - for (1,2,3) { - system(@pperl_cmd); - ok (!$?, "Run in persistent env (PPerl pass $_): exit $?"); - } - - ok (! system (@pperl_term_cmd), 'killed pperl instance'); - } - - # try with speedy-cgi - SKIP: { - skip 'SPeedyCGI persistent environment tests require CGI::SpeedyCGI', 1 - unless eval { require CGI::SpeedyCGI }; + my @cmd = @{$persistence_tests->{$type}{cmd}}; + # since PPerl is racy and sucks - just prime the "server" { local $ENV{DBICTEST_PERSISTENT_ENV_BAIL_EARLY} = 1; - skip "Something is wrong with speedy ($!)", 1 - if system(@scgi_cmd); + system(@cmd); sleep 1; + + # see if the thing actually runs, if not - might as well bail now + skip "Something is wrong with $type ($!)", 1 + if system(@cmd); } for (1,2,3) { - system(@scgi_cmd); - ok (!$?, "Run in persistent env (SpeedyCGI pass $_): exit $?"); + note ("Starting run in persistent env ($type pass $_)"); + IPC::Open2::open2(my $out, undef, @cmd); + my @out_lines; + while (my $ln = <$out>) { + next if $ln =~ /^\s*$/; + push @out_lines, " $ln"; + last if $ln =~ /^\d+\.\.\d+$/; # this is persistence, we need to terminate reading on our end + } + print $_ for @out_lines; + close $out; + wait; + ok (!$?, "Run in persistent env ($type pass $_): exit $?"); + ok (scalar @out_lines, "Run in persistent env ($type pass $_): got output"); } - } + + ok (! system (@{$persistence_tests->{$type}{termcmd}}), "killed $type server instance") + if $persistence_tests->{$type}{termcmd}; + }} } done_testing; @@ -415,9 +486,9 @@ done_testing; # PID files to go by (man does pperl really suck :( END { unless ($ENV{DBICTEST_IN_PERSISTENT_ENV}) { - close STDOUT; - close STDERR; + close $_ for (*STDIN, *STDOUT, *STDERR); local $?; # otherwise test will inherit $? of the system() - system (@pperl_term_cmd); + system (@{$persistence_tests->{PPerl}{termcmd}}) + if $persistence_tests->{PPerl}{termcmd}; } }