From: Peter Rabbitson Date: Tue, 26 Oct 2010 11:16:26 +0000 (+0200) Subject: Temporary fixes for 5.13.x $@ handling X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=1f870d5a08fa1794734380d7c8bd6753cdcf8f6d;p=dbsrgits%2FDBIx-Class-Historic.git Temporary fixes for 5.13.x $@ handling Some patchups to mitigate fallout from http://perl5.git.perl.org/perl.git/commitdiff/96d9b9cd Included several "time-bombs" set to go off when 5.13.8 is available (the "Contentious Changes freeze" release) --- diff --git a/Changes b/Changes index 4a609e2..a9ae59e 100644 --- a/Changes +++ b/Changes @@ -31,6 +31,8 @@ Revision history for DBIx::Class * Fixes - Fix memory leak during populate() on 5.8.x perls + - Temporarily fixed 5.13.x failures (RT#58225) + (perl-core fix still pending) - Fix result_soutrce_instance leaks on compose_namespace - Make sure exception_action does not allow exception-hiding due to badly-written handlers (the mechanism was never meant diff --git a/lib/DBIx/Class/Storage/TxnScopeGuard.pm b/lib/DBIx/Class/Storage/TxnScopeGuard.pm index f686cce..4365b9d 100644 --- a/lib/DBIx/Class/Storage/TxnScopeGuard.pm +++ b/lib/DBIx/Class/Storage/TxnScopeGuard.pm @@ -4,14 +4,69 @@ use strict; use warnings; use Carp::Clan qw/^DBIx::Class/; use Try::Tiny; -use Scalar::Util qw/weaken/; +use Scalar::Util qw/weaken blessed/; +use DBIx::Class::Exception; use namespace::clean; +# temporary until we fix the $@ issue in core +# we also need a real appendable, stackable exception object +# (coming soon) +BEGIN { + if ($] < 5.013001) { + *IS_BROKEN_PERL = sub () { 0 }; + } + elsif ($] < 5.013008) { + *IS_BROKEN_PERL = sub () { 1 }; + } + else { + die 'The $@ debacle should have been resolved by now, adjust DBIC'; + } +} + +my ($guards_count, $compat_handler, $foreign_handler); + sub new { my ($class, $storage) = @_; $storage->txn_begin; my $guard = bless [ 0, $storage, $storage->_dbh ], ref $class || $class; + + + # install a callback carefully + if (IS_BROKEN_PERL and !$guards_count) { + + # if the thrown exception is a plain string, wrap it in our + # own exception class + # this is actually a pretty cool idea, may very well keep it + # after perl is fixed + $compat_handler ||= bless( + sub { + $@ = (blessed($_[0]) or ref($_[0])) + ? $_[0] + : bless ( { msg => $_[0] }, 'DBIx::Class::Exception') + ; + die; + }, + '__TxnScopeGuard__FIXUP__', + ); + + if ($foreign_handler = $SIG{__DIE__}) { + $SIG{__DIE__} = bless ( + sub { + # we trust the foreign handler to do whatever it wants, all we do is set $@ + eval { $compat_handler->(@_) }; + $foreign_handler->(@_); + }, + '__TxnScopeGuard__FIXUP__', + ); + } + else { + $SIG{__DIE__} = $compat_handler; + } + } + + $guards_count++; + weaken ($guard->[2]); $guard; } @@ -26,6 +81,29 @@ sub commit { sub DESTROY { my ($dismiss, $storage) = @{$_[0]}; + $guards_count--; + + # don't touch unless it's ours, and there are no more of us left + if ( + IS_BROKEN_PERL + and + !$guards_count + ) { + + if (ref $SIG{__DIE__} eq '__TxnScopeGuard__FIXUP__') { + # restore what we saved + if ($foreign_handler) { + $SIG{__DIE__} = $foreign_handler; + } + else { + delete $SIG{__DIE__}; + } + } + + # make sure we do not leak the foreign one in case it exists + undef $foreign_handler; + } + return if $dismiss; # if our dbh is not ours anymore, the weakref will go undef @@ -47,7 +125,13 @@ sub DESTROY { catch { $rollback_exception = shift }; if (defined $rollback_exception && $rollback_exception !~ /DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION/) { - if ($exception) { + # append our text - THIS IS A TEMPORARY FIXUP! + # a real stackable exception object is in the works + if (ref $exception eq 'DBIx::Class::Exception') { + $exception->{msg} = "Transaction aborted: $exception->{msg} " + ."Rollback failed: ${rollback_exception}"; + } + elsif ($exception) { $exception = "Transaction aborted: ${exception} " ."Rollback failed: ${rollback_exception}"; } @@ -62,7 +146,7 @@ sub DESTROY { } } - $@ = $exception; + $@ = $exception unless IS_BROKEN_PERL; } 1; diff --git a/t/52leaks.t b/t/52leaks.t index 2f1867d..ad781ac 100644 --- a/t/52leaks.t +++ b/t/52leaks.t @@ -165,6 +165,10 @@ for my $slot (keys %$weak_registry) { delete $weak_registry->{$slot} unless $cleared->{hash_merge_singleton}{$weak_registry->{$slot}{weakref}{behavior}}++; } + elsif ($slot =~ /^__TxnScopeGuard__FIXUP__/) { + die 'The $@ debacle should have been fixed by now!!!' if $] >= 5.013008; + delete $weak_registry->{$slot}; + } } diff --git a/xt/podcoverage.t b/xt/podcoverage.t index 8c9132e..ba30f31 100644 --- a/xt/podcoverage.t +++ b/xt/podcoverage.t @@ -47,6 +47,11 @@ my $exceptions = { MULTICREATE_DEBUG /], }, + 'DBIx::Class::Storage::TxnScopeGuard' => { + ignore => [qw/ + IS_BROKEN_PERL + /], + }, 'DBIx::Class::FilterColumn' => { ignore => [qw/ new