Fix embarrassing leak triggered on calling new_related on an uninserted object

Reason is a typo-ed hash element in a weaken() call back in 68f3b0dd. Add a
bunch of extra tests to catch this, and also better validate the new_related
behavior.

diff --git a/Changes b/Changes
index 1e3dfbd..7b3d781 100644 (file)
--- a/Changes
+++ b/Changes
@@ -13,6 +13,7 @@ Revision history for DBIx::Class
         - A number of corner case fixes of void context populate() with \[]
         - Fix corner case of forked children disconnecting the parents DBI
           handle
+        - Fix leakage of $schema on in-memory new_related() calls
 
     * Misc
         - Codebase is now trailing-whitespace-free

diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm
index 2b43e47..fdbec40 100644 (file)
--- a/lib/DBIx/Class/Relationship/Base.pm
+++ b/lib/DBIx/Class/Relationship/Base.pm
@@ -461,11 +461,9 @@ sub related_resultset {
         my $reverse = $source->reverse_relationship_info($rel);
         foreach my $rev_rel (keys %$reverse) {
           if ($reverse->{$rev_rel}{attrs}{accessor} && $reverse->{$rev_rel}{attrs}{accessor} eq 'multi') {
-            $attrs->{related_objects}{$rev_rel} = [ $self ];
-            weaken $attrs->{related_object}{$rev_rel}[0];
+            weaken($attrs->{related_objects}{$rev_rel}[0] = $self);
           } else {
-            $attrs->{related_objects}{$rev_rel} = $self;
-            weaken $attrs->{related_object}{$rev_rel};
+            weaken($attrs->{related_objects}{$rev_rel} = $self);
           }
         }
       }

diff --git a/t/52leaks.t b/t/52leaks.t
index 5614252..ea055a6 100644 (file)
--- a/t/52leaks.t
+++ b/t/52leaks.t
@@ -179,9 +179,43 @@ my @compose_ns_classes;
     $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' ] } ),
@@ -193,7 +227,6 @@ my @compose_ns_classes;
     result_source_handle => $rs->result_source->handle,
 
     pager_explicit_count => $pager_explicit_count,
-
   };
 
   require Storable;
@@ -201,6 +234,7 @@ my @compose_ns_classes;
     %$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,

diff --git a/t/multi_create/in_memory.t b/t/multi_create/in_memory.t
index f1bb0d7..25d290a 100644 (file)
--- a/t/multi_create/in_memory.t
+++ b/t/multi_create/in_memory.t
@@ -23,22 +23,20 @@ my $schema = DBICTest->init_schema();
 {
     my $new_artist = $schema->resultset("Artist")->new_result({ 'name' => 'Depeche Mode' });
     my $new_related_cd = $new_artist->new_related('cds', { 'title' => 'Leave in Silence', 'year' => 1982});
-    eval {
+    lives_ok {
         $new_artist->insert;
         $new_related_cd->insert;
-    };
-    is ($@, '', 'Staged insertion successful');
+    } 'Staged insertion successful';
     ok($new_artist->in_storage, 'artist inserted');
     ok($new_related_cd->in_storage, 'new_related_cd inserted');
 }
 
 {
-    my $new_artist = $schema->resultset("Artist")->new_result({ 'name' => 'Depeche Mode' });
+    my $new_artist = $schema->resultset("Artist")->new_result({ 'name' => 'Mode Depeche' });
     my $new_related_cd = $new_artist->new_related('cds', { 'title' => 'Leave Slightly Noisily', 'year' => 1982});
-    eval {
+    lives_ok {
         $new_related_cd->insert;
-    };
-    is ($@, '', 'CD insertion survives by finding artist');
+    } 'CD insertion survives by finding artist';
     ok($new_artist->in_storage, 'artist inserted');
     ok($new_related_cd->in_storage, 'new_related_cd inserted');
 }
@@ -48,10 +46,9 @@ my $schema = DBICTest->init_schema();
     my $new_artist = $schema->resultset("Artist")->new ({ 'name' => 'Depeche Mode 2: Insertion Boogaloo' });
     $new_cd->artist ($new_artist);
 
-    eval {
+    lives_ok {
         $new_cd->insert;
-    };
-    is ($@, '', 'CD insertion survives by inserting artist');
+    } 'CD insertion survives by inserting artist';
     ok($new_cd->in_storage, 'new_related_cd inserted');
     ok($new_artist->in_storage, 'artist inserted');
 
@@ -60,6 +57,21 @@ my $schema = DBICTest->init_schema();
     is ($retrieved_cd->artist->name, 'Depeche Mode 2: Insertion Boogaloo', 'Correct artist attached to cd');
 }
 
+{
+    my $new_cd = $schema->resultset('CD')->new ({ 'title' => 'Leave screaming Off Key in the nude', 'year' => 1982});
+    my $new_related_artist = $new_cd->new_related( artist => { 'name' => 'Depeche Mode 3: Insertion Boogaloo' });
+    lives_ok {
+        $new_related_artist->insert;
+        $new_cd->insert;
+    } 'CD insertion survives after inserting artist';
+    ok($new_cd->in_storage, 'cd inserted');
+    ok($new_related_artist->in_storage, 'artist inserted');
+
+    my $retrieved_cd = $schema->resultset('CD')->find ({ 'title' => 'Leave screaming Off Key in the nude'});
+    ok ($retrieved_cd, 'CD found in db');
+    is ($retrieved_cd->artist->name, 'Depeche Mode 3: Insertion Boogaloo', 'Correct artist attached to cd');
+}
+
 # test both sides of a 1:(1|0)
 {
   for my $reldir ('might_have', 'belongs_to') {
@@ -102,13 +114,13 @@ my $schema = DBICTest->init_schema();
       [
         {
           artist => 1,
-          cdid => 9,
+          cdid => 10,
           genreid => undef,
           single_track => undef,
           title => "$reldir: Latest cd",
           tracks => [
             {
-              cd => 9,
+              cd => 10,
               last_updated_at => undef,
               last_updated_on => undef,
               position => 1,
@@ -120,7 +132,7 @@ my $schema = DBICTest->init_schema();
         },
         {
           artist => 1,
-          cdid => 10,
+          cdid => 11,
           genreid => undef,
           single_track => 19,
           title => "$reldir: Awesome first single",