From 44819b47179936492c093811ed0f625ce6e029a3 Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <david@loudthinking.com>
Date: Wed, 22 Dec 2004 00:48:24 +0000
Subject: [PATCH] Fixed that nested transactions now work by letting the outer
 most transaction have the responsibilty of starting and rolling back the
 transaction. If any of the inner transactions swallow the exception raised,
 though, the transaction will not be rolled back. So always let the
 transaction bubble up even when you've dealt with local issues. Closes #231
 and #340.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@242 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
---
 activerecord/CHANGELOG                        |  4 ++++
 .../connection_adapters/abstract_adapter.rb   |  8 +++----
 .../lib/active_record/transactions.rb         | 24 +++++++++----------
 activerecord/test/transactions_test.rb        |  2 +-
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 7e26b3c10a..a82f8a0013 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,9 @@
 *SVN*
 
+* Fixed that nested transactions now work by letting the outer most transaction have the responsibilty of starting and rolling back the transaction.
+  If any of the inner transactions swallow the exception raised, though, the transaction will not be rolled back. So always let the transaction
+  bubble up even when you've dealt with local issues. Closes #231 and #340.
+
 * Fixed validates_{confirmation,acceptance}_of to only happen when the virtual attributes are not nil #348 [dpiddy@gmail.com]
 
 * Added a require_association hook on const_missing that makes it possible to use any model class without requiring it first. This makes STI look like:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index b521a0fdf8..d41356ffa4 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -298,16 +298,16 @@ def reset_runtime # :nodoc:
       end
 
       # Wrap a block in a transaction.  Returns result of block.
-      def transaction
+      def transaction(start_db_transaction = true)
         begin
           if block_given?
-            begin_db_transaction
+            begin_db_transaction if start_db_transaction
             result = yield
-            commit_db_transaction
+            commit_db_transaction if start_db_transaction
             result
           end
         rescue Exception => database_transaction_rollback
-          rollback_db_transaction
+          rollback_db_transaction if start_db_transaction
           raise
         end
       end
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index d440e74346..16c56c58f0 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -77,13 +77,17 @@ def self.append_features(base)
     # Tribute: Object-level transactions are implemented by Transaction::Simple by Austin Ziegler.
     module ClassMethods      
       def transaction(*objects, &block)
-        TRANSACTION_MUTEX.lock
+        TRANSACTION_MUTEX.synchronize do
+          Thread.current['open_transactions'] ||= 0
+          Thread.current['start_db_transaction'] = (Thread.current['open_transactions'] == 0)
+          Thread.current['open_transactions'] += 1
+        end
 
         begin
           objects.each { |o| o.extend(Transaction::Simple) }
           objects.each { |o| o.start_transaction }
 
-          result = connection.transaction(&block)
+          result = connection.transaction(Thread.current['start_db_transaction'], &block)
 
           objects.each { |o| o.commit_transaction }
           return result
@@ -91,7 +95,9 @@ def transaction(*objects, &block)
           objects.each { |o| o.abort_transaction }
           raise
         ensure
-          TRANSACTION_MUTEX.unlock
+          TRANSACTION_MUTEX.synchronize do
+            Thread.current['open_transactions'] -= 1
+          end
         end
       end
     end
@@ -101,19 +107,11 @@ def transaction(*objects, &block)
     end
 
     def destroy_with_transactions #:nodoc:
-      if TRANSACTION_MUTEX.locked?
-        destroy_without_transactions
-      else
-        transaction { destroy_without_transactions }
-      end
+      transaction { destroy_without_transactions }
     end
     
     def save_with_transactions(perform_validation = true) #:nodoc:
-      if TRANSACTION_MUTEX.locked?
-        save_without_transactions(perform_validation)
-      else
-        transaction { save_without_transactions(perform_validation) }
-      end
+      transaction { save_without_transactions(perform_validation) }
     end
   end
 end
diff --git a/activerecord/test/transactions_test.rb b/activerecord/test/transactions_test.rb
index 18b2ea3e65..cf10b7d3ee 100644
--- a/activerecord/test/transactions_test.rb
+++ b/activerecord/test/transactions_test.rb
@@ -84,7 +84,7 @@ def test_callback_rollback_in_save
     end
   end
 
-  def xtest_nested_explicit_transactions
+  def test_nested_explicit_transactions
     Topic.transaction do
       Topic.transaction do
         @first.approved  = 1
-- 
GitLab