Avoid infinite loop if save point does not exist

( cherry-pick of f5f0cb1dd )

diff --git a/AUTHORS b/AUTHORS
index 6afe2ba..2d26034 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -159,6 +159,7 @@ Peter Valdemar Mørch <peter@morch.com>
 peter: Peter Collingbourne <peter@pcc.me.uk>
 phaylon: Robert Sedlacek <phaylon@dunkelheit.at>
 plu: Johannes Plunien <plu@cpan.org>
+pmooney: Paul Mooney <paul.mooney@net-a-porter.com>
 Possum: Daniel LeWarne <possum@cpan.org>
 pplu: Jose Luis Martinez <jlmartinez@capside.com>
 quicksilver: Jules Bean <jules@jellybean.co.uk>

diff --git a/Changes b/Changes
index 92c57f4..ba48d7d 100644 (file)
--- a/Changes
+++ b/Changes
@@ -18,6 +18,7 @@ Revision history for DBIx::Class
         - Fix silencing of exceptions thrown by custom inflate_result() methods
         - Fix complex prefetch when ordering over foreign boolean columns
           ( Pg can't MAX(boolcol) despite being able to ORDER BY boolcol )
+        - Fix infinite loop on ->svp_release("nonexistent_savepoint") (GH#97)
         - Fix spurious ROLLBACK statements when a TxnScopeGuard fails a commit
           of a transaction with deferred FK checks: a guard is now inactivated
           immediately before the commit is attempted (RT#107159)

diff --git a/lib/DBIx/Class/Storage.pm b/lib/DBIx/Class/Storage.pm
index 049230a..97e84d8 100644 (file)
--- a/lib/DBIx/Class/Storage.pm
+++ b/lib/DBIx/Class/Storage.pm
@@ -328,12 +328,15 @@ sub svp_release {
 
   if (defined $name) {
     my @stack = @{ $self->savepoints };
-    my $svp;
+    my $svp = '';
 
-    do { $svp = pop @stack } until $svp eq $name;
+    while( $svp ne $name ) {
 
-    $self->throw_exception ("Savepoint '$name' does not exist")
-      unless $svp;
+      $self->throw_exception ("Savepoint '$name' does not exist")
+        unless @stack;
+
+      $svp = pop @stack;
+    }
 
     $self->savepoints(\@stack); # put back what's left
   }

diff --git a/t/storage/savepoints.t b/t/storage/savepoints.t
index b42ceca..eba3bce 100644 (file)
--- a/t/storage/savepoints.t
+++ b/t/storage/savepoints.t
@@ -3,6 +3,9 @@ use warnings;
 
 use Test::More;
 use Test::Exception;
+use DBIx::Class::Optional::Dependencies;
+use DBIx::Class::_Util qw(sigwarn_silencer scope_guard);
+use Scalar::Util 'weaken';
 
 use lib qw(t/lib);
 use DBICTest;
@@ -231,6 +234,17 @@ for ('', keys %$env2optdep) { SKIP: {
   # make sure a fresh txn will work after above
   $schema->storage->txn_do(sub { ok "noop" } );
 
+### Make sure non-existend savepoint release doesn't infloop itself
+  {
+    weaken( my $s = $schema );
+
+    throws_ok {
+      $s->storage->txn_do(sub { $s->svp_release('wibble') })
+    } qr/Savepoint 'wibble' does not exist/,
+      "Calling svp_release on a non-existant savepoint throws expected error"
+    ;
+  }
+
 ### cleanupz
   $schema->storage->dbh->do ("DROP TABLE artist");
 }}