X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=lib%2FDBIx%2FClass%2FStorage%2FDBI.pm;h=d390dc6858158b7745ac03a86a7457aac0b69608;hb=86cdddbe2781f77d81d27cdd83910543c313a8f0;hp=af27c55fe348d07e20fd16bf714b4b197338f229;hpb=63af9ced21d59d5d92099a9b8853c2eece90569a;p=dbsrgits%2FDBIx-Class.git

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index af27c55..d390dc6 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -13,7 +13,11 @@ use List::Util qw/first/;
 use Context::Preserve 'preserve_context';
 use Try::Tiny;
 use SQL::Abstract qw(is_plain_value is_literal_value);
-use DBIx::Class::_Util qw(quote_sub perlstring serialize detected_reinvoked_destructor);
+use DBIx::Class::_Util qw(
+  quote_sub perlstring serialize
+  dbic_internal_try
+  detected_reinvoked_destructor scope_guard
+);
 use namespace::clean;
 
 # default cursor class, overridable in connect_info attributes
@@ -224,13 +228,17 @@ sub new {
   }
 
   END {
-    local $?; # just in case the DBI destructor changes it somehow
 
-    # destroy just the object if not native to this process
-    $_->_verify_pid for (grep
-      { defined $_ }
-      values %seek_and_destroy
-    );
+    if(
+      ! DBIx::Class::_ENV_::BROKEN_FORK
+        and
+      my @instances = grep { defined $_ } values %seek_and_destroy
+    ) {
+      local $?; # just in case the DBI destructor changes it somehow
+
+      # disarm the handle if not native to this process (see comment on top)
+      $_->_verify_pid for @instances;
+    }
   }
 
   sub CLONE {
@@ -242,9 +250,7 @@ sub new {
 
     for (@instances) {
       $_->_dbh(undef);
-
-      $_->transaction_depth(0);
-      $_->savepoints([]);
+      $_->disconnect;
 
       # properly renumber existing refs
       $_->_arm_global_destructor
@@ -256,9 +262,13 @@ sub DESTROY {
   return if &detected_reinvoked_destructor;
 
   $_[0]->_verify_pid unless DBIx::Class::_ENV_::BROKEN_FORK;
+
   # some databases spew warnings on implicit disconnect
+  return unless defined $_[0]->_dbh;
+
   local $SIG{__WARN__} = sub {};
   $_[0]->_dbh(undef);
+  # not calling ->disconnect here - we are being destroyed - nothing to reset
 
   # this op is necessary, since the very last perl runtime statement
   # triggers a global destruction shootout, and the $SIG localization
@@ -275,8 +285,7 @@ sub _verify_pid {
   if( defined $pid and $pid != $$ and my $dbh = $_[0]->_dbh ) {
     $dbh->{InactiveDestroy} = 1;
     $_[0]->_dbh(undef);
-    $_[0]->transaction_depth(0);
-    $_[0]->savepoints([]);
+    $_[0]->disconnect;
   }
 
   return;
@@ -870,20 +879,37 @@ database is not in C<AutoCommit> mode.
 =cut
 
 sub disconnect {
+  my $self = shift;
+
+  # this physical disconnect below might very well throw
+  # in order to unambiguously reset the state - do the cleanup in guard
+
+  my $g = scope_guard {
+
+    {
+      local $@ if DBIx::Class::_ENV_::UNSTABLE_DOLLARAT;
+      eval { $self->_dbh->disconnect };
+    }
+
+    $self->_dbh(undef);
+    $self->_dbh_details({});
+    $self->transaction_depth(undef);
+    $self->_dbh_autocommit(undef);
+    $self->savepoints([]);
+
+    # FIXME - this needs reenabling with the proper "no reset on same DSN" check
+    #$self->_sql_maker(undef); # this may also end up being different
+  };
 
-  if( my $dbh = $_[0]->_dbh ) {
+  if( $self->_dbh ) {
 
-    $_[0]->_do_connection_actions(disconnect_call_ => $_) for (
-      ( $_[0]->on_disconnect_call || () ),
-      $_[0]->_parse_connect_do ('on_disconnect_do')
+    $self->_do_connection_actions(disconnect_call_ => $_) for (
+      ( $self->on_disconnect_call || () ),
+      $self->_parse_connect_do ('on_disconnect_do')
     );
 
     # stops the "implicit rollback on disconnect" warning
-    $_[0]->_exec_txn_rollback unless $_[0]->_dbh_autocommit;
-
-    %{ $dbh->{CachedKids} } = ();
-    $dbh->disconnect;
-    $_[0]->_dbh(undef);
+    $self->_exec_txn_rollback unless $self->_dbh_autocommit;
   }
 }
 
@@ -936,7 +962,15 @@ sub connected {
 sub _seems_connected {
   $_[0]->_verify_pid unless DBIx::Class::_ENV_::BROKEN_FORK;
 
-  ($_[0]->_dbh || return 0)->FETCH('Active');
+  $_[0]->_dbh
+    and
+  $_[0]->_dbh->FETCH('Active')
+    and
+  return 1;
+
+  # explicitly reset all state
+  $_[0]->disconnect;
+  return 0;
 }
 
 sub _ping {
@@ -1038,12 +1072,9 @@ sub _init {}
 
 sub _populate_dbh {
 
-  $_[0]->_dbh(undef); # in case ->connected failed we might get sent here
-
-  $_[0]->_dbh_details({}); # reset everything we know
-
-  # FIXME - this needs reenabling with the proper "no reset on same DSN" check
-  #$_[0]->_sql_maker(undef); # this may also end up being different
+  # reset internal states
+  # also in case ->connected failed we might get sent here
+  $_[0]->disconnect;
 
   $_[0]->_dbh($_[0]->_connect);
 
@@ -1053,7 +1084,7 @@ sub _populate_dbh {
 
   # Always set the transaction depth on connect, since
   #  there is no transaction in progress by definition
-  $_[0]->{transaction_depth} = $_[0]->_dbh_autocommit ? 0 : 1;
+  $_[0]->transaction_depth( $_[0]->_dbh_autocommit ? 0 : 1 );
 
   $_[0]->_run_connection_actions unless $_[0]->{_in_determine_driver};
 
@@ -1121,7 +1152,7 @@ sub _server_info {
 
     my $info = {};
 
-    my $server_version = try {
+    my $server_version = dbic_internal_try {
       $self->_get_server_version
     } catch {
       # driver determination *may* use this codepath
@@ -1182,7 +1213,7 @@ sub _describe_connection {
   my $self = shift;
 
   my $drv;
-  try {
+  dbic_internal_try {
     $drv = $self->_extract_driver_from_connect_info;
     $self->ensure_connected;
   };
@@ -1196,7 +1227,7 @@ sub _describe_connection {
     DBIC_DRIVER => ref $self,
     $drv ? (
       DBD => $drv,
-      DBD_VER => try { $drv->VERSION },
+      DBD_VER => dbic_internal_try { $drv->VERSION },
     ) : (),
   };
 
@@ -1237,7 +1268,7 @@ sub _describe_connection {
   ) {
     # some drivers barf on things they do not know about instead
     # of returning undef
-    my $v = try { $self->_dbh_get_info($inf) };
+    my $v = dbic_internal_try { $self->_dbh_get_info($inf) };
     next unless defined $v;
 
     #my $key = sprintf( '%s(%s)', $inf, $DBI::Const::GetInfoType::GetInfoType{$inf} );
@@ -1379,7 +1410,7 @@ sub _warn_undetermined_driver {
 sub _do_connection_actions {
   my ($self, $method_prefix, $call, @args) = @_;
 
-  try {
+  dbic_internal_try {
     if (not ref($call)) {
       my $method = $method_prefix . $call;
       $self->$method(@args);
@@ -1405,6 +1436,7 @@ sub _do_connection_actions {
       # a handle in an undefined state in our storage object
       # kill it with fire and rethrow
       $self->_dbh(undef);
+      $self->disconnect;  # the $dbh is gone, but we still need to reset the rest
       $self->throw_exception( $_[0] );
     }
     else {
@@ -1502,7 +1534,7 @@ sub _connect {
     }, '__DBIC__DBH__ERROR__HANDLER__';
   };
 
-  try {
+  dbic_internal_try {
     if(ref $info->[0] eq 'CODE') {
       $dbh = $info->[0]->();
     }
@@ -1618,7 +1650,9 @@ sub _exec_txn_commit {
 sub txn_rollback {
   my $self = shift;
 
-  $self->throw_exception("Unable to txn_rollback() on a disconnected storage")
+  # do a minimal connectivity check due to weird shit like
+  # https://rt.cpan.org/Public/Bug/Display.html?id=62370
+  $self->throw_exception("lost connection to storage")
     unless $self->_seems_connected;
 
   # esoteric case for folks using external $dbh handles
@@ -1970,7 +2004,7 @@ sub insert {
   if (my $retlist = $sqla_opts->{returning}) {  # if IR is supported - we will get everything in one set
 
     unless( @ir_container ) {
-      try {
+      dbic_internal_try {
 
         # FIXME - need to investigate why Caelum silenced this in 4d4dc518
         local $SIG{__WARN__} = sub {};
@@ -2315,7 +2349,7 @@ sub _dbh_execute_for_fetch {
 
   my $tuple_status = [];
   my ($rv, $err);
-  try {
+  dbic_internal_try {
     $rv = $sth->execute_for_fetch(
       $fetch_tuple,
       $tuple_status,
@@ -2334,7 +2368,7 @@ sub _dbh_execute_for_fetch {
   );
 
   # Statement must finish even if there was an exception.
-  try {
+  dbic_internal_try {
     $sth->finish
   }
   catch {
@@ -2362,7 +2396,7 @@ sub _dbh_execute_inserts_with_no_binds {
   my ($self, $sth, $count) = @_;
 
   my $err;
-  try {
+  dbic_internal_try {
     my $dbh = $self->_get_dbh;
     local $dbh->{RaiseError} = 1;
     local $dbh->{PrintError} = 0;
@@ -2374,7 +2408,7 @@ sub _dbh_execute_inserts_with_no_binds {
   };
 
   # Make sure statement is finished even if there was an exception.
-  try {
+  dbic_internal_try {
     $sth->finish
   }
   catch {
@@ -2473,7 +2507,7 @@ sub _select_args {
       and
     @{$attrs->{group_by}}
       and
-    my $grp_aliases = try { # try{} because $attrs->{from} may be unreadable
+    my $grp_aliases = dbic_internal_try { # internal_try{} because $attrs->{from} may be unreadable
       $self->_resolve_aliastypes_from_select_args({ from => $attrs->{from}, group_by => $attrs->{group_by} })
     }
   ) {
@@ -2590,7 +2624,7 @@ sub _dbh_columns_info_for {
   my %result;
 
   if (! DBIx::Class::_ENV_::STRESSTEST_COLUMN_INFO_UNAWARE_STORAGE and $dbh->can('column_info')) {
-    try {
+    dbic_internal_try {
       my ($schema,$tab) = $table =~ /^(.+?)\.(.+)$/ ? ($1,$2) : (undef,$table);
       my $sth = $dbh->column_info( undef,$schema, $tab, '%' );
       $sth->execute();
@@ -2694,7 +2728,7 @@ Return the row id of the last insert.
 sub _dbh_last_insert_id {
     my ($self, $dbh, $source, $col) = @_;
 
-    my $id = try { $dbh->last_insert_id (undef, undef, $source->name, $col) };
+    my $id = dbic_internal_try { $dbh->last_insert_id (undef, undef, $source->name, $col) };
 
     return $id if defined $id;
 
@@ -2745,15 +2779,15 @@ sub _determine_supports_placeholders {
 
   # some drivers provide a $dbh attribute (e.g. Sybase and $dbh->{syb_dynamic_supported})
   # but it is inaccurate more often than not
-  return try {
+  ( dbic_internal_try {
     local $dbh->{PrintError} = 0;
     local $dbh->{RaiseError} = 1;
     $dbh->do('select ?', {}, 1);
     1;
-  }
-  catch {
-    0;
-  };
+  } )
+    ? 1
+    : 0
+  ;
 }
 
 # Check if placeholders bound to non-string types throw exceptions
@@ -2762,16 +2796,16 @@ sub _determine_supports_typeless_placeholders {
   my $self = shift;
   my $dbh  = $self->_get_dbh;
 
-  return try {
+  ( dbic_internal_try {
     local $dbh->{PrintError} = 0;
     local $dbh->{RaiseError} = 1;
     # this specifically tests a bind that is NOT a string
     $dbh->do('select 1 where 1 = ?', {}, 1);
     1;
-  }
-  catch {
-    0;
-  };
+  } )
+    ? 1
+    : 0
+  ;
 }
 
 =head2 sqlt_type
@@ -3107,7 +3141,7 @@ sub deploy {
     return if($line =~ /^COMMIT/m);
     return if $line =~ /^\s+$/; # skip whitespace only
     $self->_query_start($line);
-    try {
+    dbic_internal_try {
       # do a dbh_do cycle here, as we need some error checking in
       # place (even though we will ignore errors)
       $self->dbh_do (sub { $_[1]->do($line) });