Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Backport Bugfix: Stack Overflow (3-0-stable) #5044

Merged
merged 2 commits into from

5 participants

@AntiTyping

Bugfix for #2525 belongs_to + :inverse_of + accepts_nested_attributes_for causes stack overflow on save

I run into the same issue. There is a cyclic autosave for belongs_to/has_one when using accepts_nested_attributes. It causes stack overflow.

@tenderlove
Owner

Do you mind adding a test for this? Also, does it occur on the master branch?

@AntiTyping

1) I added a test.
2) I just verified that the issue does not occur on the master branch.

@carlosantoniodasilva

@jonleighton @tenderlove should this be applied to 3-0, could you please check? ^^

@tenderlove tenderlove commented on the diff
activerecord/lib/active_record/autosave_association.rb
@@ -140,6 +140,23 @@ def #{type}(name, options = {})
CODE
end
+ def define_non_cyclic_method(name, reflection, &block)
+ define_method(name) do |*args|
+ result = true; @_already_called ||= {}
@tenderlove Owner

Does this need to be an instance variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@wycats wycats merged commit 51582fe into rails:3-0-stable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
19 activerecord/lib/active_record/autosave_association.rb
@@ -140,6 +140,23 @@ def #{type}(name, options = {})
CODE
end
+ def define_non_cyclic_method(name, reflection, &block)
+ define_method(name) do |*args|
+ result = true; @_already_called ||= {}
@tenderlove Owner

Does this need to be an instance variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ # Loop prevention for validation of associations
+ unless @_already_called[[name, reflection.name]]
+ begin
+ @_already_called[[name, reflection.name]]=true
+ result = instance_eval(&block)
+ ensure
+ @_already_called[[name, reflection.name]]=false
+ end
+ end
+
+ result
+ end
+ end
+
# Adds validation and save callbacks for the association as specified by
# the +reflection+.
#
@@ -169,7 +186,7 @@ def add_autosave_association_callbacks(reflection)
define_method(save_method) { save_has_one_association(reflection) }
after_save save_method
else
- define_method(save_method) { save_belongs_to_association(reflection) }
+ define_non_cyclic_method(save_method, reflection) { save_belongs_to_association(reflection) }
before_save save_method
end
end
View
12 activerecord/test/cases/autosave_association_test.rb
@@ -3,8 +3,10 @@
require 'models/company'
require 'models/customer'
require 'models/developer'
+require 'models/face'
require 'models/invoice'
require 'models/line_item'
+require 'models/man'
require 'models/order'
require 'models/parrot'
require 'models/person'
@@ -880,6 +882,16 @@ def test_should_not_load_the_associated_model
end
end
+class TestAutosaveInverseAssociationOnAHasOneAssociation < ActiveRecord::TestCase
+ self.use_transactional_fixtures = false
+
+ def test_should_save_the_inverse_association_model
+ man = Man.new
+ man.build_face
+ man.face.save
+ end
+end
+
class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
self.use_transactional_fixtures = false
View
2  activerecord/test/models/face.rb
@@ -4,4 +4,6 @@ class Face < ActiveRecord::Base
# These is a "broken" inverse_of for the purposes of testing
belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face
belongs_to :horrible_polymorphic_man, :polymorphic => true, :inverse_of => :horrible_polymorphic_face
+
+ accepts_nested_attributes_for :man
end
Something went wrong with that request. Please try again.