Reduced memory leak problem in transactions by lazily updating AR objects with new transaction state. If AR object has a callback, the callback will be performed immediately (non-lazily) so the transaction still has to keep records with callbacks.

6 files changed, 89 insertions, 6 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index a888b79391..e864e90e6f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,27 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Fixing issue #776.
+
+    Memory bloat in transactions is handled by having the transaction hold only
+    the AR objects which it absolutely needs to know about. These are the AR
+    objects with callbacks (they need to be updated as soon as something in the
+    transaction occurs).
+
+    All other AR objects can be updated lazily by keeping a reference to a
+    TransactionState object. If an AR object gets inside a transaction, then
+    the transaction will add its TransactionState to the AR object. When the
+    user makes a call to some attribute on an AR object (which has no
+    callbacks) associated with a transaction, the AR object will call the
+    sync_with_transaction_state method and make sure it is up to date with the
+    transaction. After it has synced with the transaction state, the AR object
+    will return the attribute that was requested.
+
+    Most of the logic in the changes are used to handle multiple transactions,
+    in which case the AR object has to recursively follow parent pointers of
+    TransactionState objects.
+
+    *John Wang*
+
 *   Descriptive error message when the necessary AR adapter gem was not found.
     Fix #7313
 
diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb
index 310f1b6e75..3e454b713a 100644
--- a/activerecord/lib/active_record/attribute_methods/primary_key.rb
+++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb
@@ -8,27 +8,32 @@ module ActiveRecord
       # Returns this record's primary key value wrapped in an Array if one is
       # available.
       def to_key
+        sync_with_transaction_state
         key = self.id
         [key] if key
       end
 
       # Returns the primary key value.
       def id
+        sync_with_transaction_state
         read_attribute(self.class.primary_key)
       end
 
       # Sets the primary key value.
       def id=(value)
+        sync_with_transaction_state
         write_attribute(self.class.primary_key, value) if self.class.primary_key
       end
 
       # Queries the primary key value.
       def id?
+        sync_with_transaction_state
         query_attribute(self.class.primary_key)
       end
 
       # Returns the primary key value before type cast.
       def id_before_type_cast
+        sync_with_transaction_state
         read_attribute_before_type_cast(self.class.primary_key)
       end
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
index 3ecef96b10..73c80a3220 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
@@ -5,7 +5,7 @@ module ActiveRecord
 
       def initialize(connection)
         @connection = connection
-        @state = TransactionState.new 
+        @state = TransactionState.new
       end
 
       def state
@@ -14,11 +14,13 @@ module ActiveRecord
     end
 
     class TransactionState
+      attr_accessor :parent
 
       VALID_STATES = Set.new([:committed, :rolledback, nil])
 
       def initialize(state = nil)
         @state = state
+        @parent = nil
       end
 
       def committed?
@@ -116,7 +118,11 @@ module ActiveRecord
       end
 
       def add_record(record)
-        records << record
+        if record.has_transactional_callbacks?
+          records << record
+        else
+          record.set_transaction_state(@state)
+        end
       end
 
       def rollback_records
@@ -188,8 +194,9 @@ module ActiveRecord
       end
 
       def perform_commit
+        @state.set_state(:committed)
+        @state.parent = parent.state
         connection.release_savepoint
-        records.each { |r| parent.add_record(r) }
       end
     end
   end
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 63a1197a56..6510433056 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -347,8 +347,54 @@ module ActiveRecord
       Hash[methods.map { |method| [method, public_send(method)] }].with_indifferent_access
     end
 
+    def set_transaction_state(state) # :nodoc:
+      @transaction_state = state
+    end
+
+    def has_transactional_callbacks? # :nodoc:
+      !_rollback_callbacks.empty? || !_commit_callbacks.empty? || !_create_callbacks.empty?
+    end
+
     private
 
+    # Updates the attributes on this particular ActiveRecord object so that
+    # if it is associated with a transaction, then the state of the AR object
+    # will be updated to reflect the current state of the transaction
+    #
+    # The @transaction_state variable stores the states of the associated
+    # transaction. This relies on the fact that a transaction can only be in
+    # one rollback or commit (otherwise a list of states would be required)
+    # Each AR object inside of a transaction carries that transaction's
+    # TransactionState.
+    #
+    # This method checks to see if the ActiveRecord object's state reflects
+    # the TransactionState, and rolls back or commits the ActiveRecord object
+    # as appropriate.
+    #
+    # Since ActiveRecord objects can be inside multiple transactions, this
+    # method recursively goes through the parent of the TransactionState and
+    # checks if the ActiveRecord object reflects the state of the object.
+    def sync_with_transaction_state
+      update_attributes_from_transaction_state(@transaction_state, 0)
+    end
+
+    def update_attributes_from_transaction_state(transaction_state, depth)
+      if transaction_state && !has_transactional_callbacks?
+        unless @reflects_state[depth]
+          if transaction_state.committed?
+            committed!
+          elsif transaction_state.rolledback?
+            rolledback!
+          end
+          @reflects_state[depth] = true
+        end
+
+        if transaction_state.parent && !@reflects_state[depth+1]
+          update_attributes_from_transaction_state(transaction_state.parent, depth+1)
+        end
+      end
+    end
+
     # Under Ruby 1.9, Array#flatten will call #to_ary (recursively) on each of the elements
     # of the array, and then rescues from the possible NoMethodError. If those elements are
     # ActiveRecord::Base's, then this triggers the various method_missing's that we have,
@@ -376,7 +422,8 @@ module ActiveRecord
       @new_record               = true
       @txn                      = nil
       @_start_transaction_state = {}
-      @transaction              = nil
+      @transaction_state        = nil
+      @reflects_state           = [false]
     end
   end
 end
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 803cae7115..066f93635a 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -69,11 +69,13 @@ module ActiveRecord
     # Returns true if this object hasn't been saved yet -- that is, a record
     # for the object doesn't exist in the data store yet; otherwise, returns false.
     def new_record?
+      sync_with_transaction_state
       @new_record
     end
 
     # Returns true if this object has been destroyed, otherwise returns false.
     def destroyed?
+      sync_with_transaction_state
       @destroyed
     end
 
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 546737b398..6d66342fa5 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -460,7 +460,7 @@ class TransactionTest < ActiveRecord::TestCase
     assert !transaction.state.committed?
 
     transaction.perform_rollback
-    
+
     assert transaction.state.rolledback?
     assert !transaction.state.committed?
   end
@@ -474,7 +474,7 @@ class TransactionTest < ActiveRecord::TestCase
     assert !transaction.state.committed?
 
     transaction.perform_commit
-    
+
     assert !transaction.state.rolledback?
     assert transaction.state.committed?
   end