AssociationCollection#to_ary should definitely dup the target! Also changed #replace which was previously incorrect, but the test passed due to the fact that to_a was not duping.

2 files changed, 10 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 7504773639..f2997b9db3 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -45,7 +45,7 @@ module ActiveRecord
       end
 
       def to_ary
-        load_target
+        load_target.dup
       end
       alias_method :to_a, :to_ary
 
@@ -289,13 +289,13 @@ module ActiveRecord
       # This will perform a diff and delete/add only records that have changed.
       def replace(other_array)
         other_array.each { |val| raise_on_type_mismatch(val) }
-
-        load_target
+        original_target = load_target.dup
 
         transaction do
           delete(@target - other_array)
 
           unless concat(other_array - @target)
+            @target = original_target
             raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the "
                                   "new records could not be saved."
           end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 3bec9c97f4..5904966ee5 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1349,4 +1349,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal reply.id, first.id
     assert_equal true, first.approved?
   end
+
+  def test_to_a_should_dup_target
+    ary    = topics(:first).replies.to_a
+    target = topics(:first).replies.target
+
+    assert_not_equal target.object_id, ary.object_id
+  end
 end