From: Peter Rabbitson <ribasushi@cpan.org>
Date: Mon, 20 Jan 2014 19:05:00 +0000 (+0100)
Subject: Stop using precomputed SQLite testdb name, fix test-end bug in replicated.t
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=9e75be92;p=dbsrgits%2FDBIx-Class-Historic.git

Stop using precomputed SQLite testdb name, fix test-end bug in replicated.t

This was an... odd one. Originally the problem manifested as a classic
var-sub-sub closure problem, leading to $db_file being captured and tripping
a more involved testcase that is coming in a subsequent commit. However while
getting rid of the reference itself was easy, this led to the outer coderef
itself being "leaked out".

The reproducing oneliner is:

~$ perl -MScalar::Util=weaken -Mstrict -Mwarnings -e '
 sub foo {
   my $rv = sub { "wtf" }; return $rv;
 }

 my $should_be_gone;

 {
   $should_be_gone = foo();
   weaken ($should_be_gone);
 }

 warn $should_be_gone;  # why is this still defined@!%#$!$#
'

Various tools indicated that it is somehow attached to the PAD of the static
foo() sub, but I could not figure out *why* exactly this is happening, nor
how to properly list it: PadWalker's closed_over() shows nothing for foo()

Thus invoking doge on the whole shebang, and moving on. SUCH WTF, WOW!

Incientally changing things to use the current SQLite filename revealed
a bug in t/storage/replicated.t, which was never noticed as it only
resulted in annoying warnings under Win32 global destroy (test
teardown).
---

diff --git a/t/lib/DBICTest.pm b/t/lib/DBICTest.pm
index 8988db9..eca0bc6 100644
--- a/t/lib/DBICTest.pm
+++ b/t/lib/DBICTest.pm
@@ -209,12 +209,24 @@ sub _database {
       );
     }
 
+    # MASSIVE FIXME - this seems necessary, but I do not yet know why
+    # without an external variable on the pad the on_connect_do cref
+    # (starting just below) is being considered a const of some sorts
+    # and persists indefinitely... wtf --ribasushi
+    my $such_var = 'very closure... much wtf... wow!!!';
+
     return ("dbi:SQLite:${db_file}", '', '', {
       AutoCommit => 1,
 
       # this is executed on every connect, and thus installs a disconnect/DESTROY
       # guard for every new $dbh
       on_connect_do => sub {
+        # MASSIVE FIXME - this seems necessary, but I do not yet know why
+        # without an external variable on the pad the on_connect_do cref
+        # (starting just above) is being considered a const of some sorts
+        # and persists indefinitely... wtf --ribasushi
+        $such_var if 0;
+
         my $storage = shift;
         my $dbh = $storage->_get_dbh;
 
@@ -233,7 +245,7 @@ sub _database {
         # set a *DBI* disconnect callback, to make sure the physical SQLite
         # file is still there (i.e. the test does not attempt to delete
         # an open database, which fails on Win32)
-        if (my $guard_cb = __mk_disconnect_guard($db_file)) {
+        if (my $guard_cb = __mk_disconnect_guard($dbh->sqlite_db_filename)) {
           $dbh->{Callbacks} = {
             connect => sub { $guard_cb->('connect') },
             disconnect => sub { $guard_cb->('disconnect') },
diff --git a/t/storage/replicated.t b/t/storage/replicated.t
index dfd3870..c480541 100644
--- a/t/storage/replicated.t
+++ b/t/storage/replicated.t
@@ -204,7 +204,7 @@ TESTSCHEMACLASSES: {
         }
     }
 
-    ## Cleanup after ourselves.  Unlink all gthe slave paths.
+    ## Cleanup after ourselves. Unlink all the slave paths.
 
     sub cleanup {
         my $self = shift @_;
@@ -912,7 +912,9 @@ is $debug{storage_type}, 'REPLICANT', "got last query from a replicant: $debug{d
 
     is $debug{storage_type}, 'REPLICANT', "got last query from a replicant: $debug{dsn}";
 }
+
 ## Delete the old database files
+$_->disconnect for values %{ $replicated->schema->storage->replicants };
 $replicated->cleanup;
 
 done_testing;