Merge pull request #16640 from mfazekas/fix_loop_in_changed_for_autosave

Fix potenital infinite recursion in changed_for_autosave?

2 files changed, 21 insertions, 4 deletions

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index fa6c5e9e8c..fcaaffb852 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -278,11 +278,18 @@ module ActiveRecord
       # go through nested autosave associations that are loaded in memory (without loading
       # any new ones), and return true if is changed for autosave
       def nested_records_changed_for_autosave?
-        self.class._reflections.values.any? do |reflection|
-          if reflection.options[:autosave]
-            association = association_instance_get(reflection.name)
-            association && Array.wrap(association.target).any?(&:changed_for_autosave?)
+        @_nested_records_changed_for_autosave_already_called ||= false
+        return false if @_nested_records_changed_for_autosave_already_called
+        begin
+          @_nested_records_changed_for_autosave_already_called = true
+          self.class._reflections.values.any? do |reflection|
+            if reflection.options[:autosave]
+              association = association_instance_get(reflection.name)
+              association && Array.wrap(association.target).any?(&:changed_for_autosave?)
+            end
           end
+        ensure
+          @_nested_records_changed_for_autosave_already_called = false
         end
       end
 
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 04d5a2869c..52765881d0 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -1030,6 +1030,16 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
     assert_equal 'The Vile Insanity', @pirate.reload.ship.name
   end
 
+  def test_changed_for_autosave_should_handle_cycles
+    @ship.pirate = @pirate
+    assert_queries(0) { @ship.save! }
+
+    @parrot = @pirate.parrots.create(name: "some_name")
+    @parrot.name="changed_name"
+    assert_queries(1) { @ship.save! }
+    assert_queries(0) { @ship.save! }
+  end
+
   def test_should_automatically_save_bang_the_associated_model
     @pirate.ship.name = 'The Vile Insanity'
     @pirate.save!