Skip to content

Commit 09264da

Browse files
committed
Merge branch 'fix_thread_safety_bug' into 0-10-stable
2 parents cb67434 + 238d792 commit 09264da

File tree

3 files changed

+57
-2
lines changed

3 files changed

+57
-2
lines changed

lib/active_model/serializer.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ def associations(include_directive = ActiveModelSerializers.default_include_dire
347347
return Enumerator.new {} unless object
348348

349349
Enumerator.new do |y|
350-
self.class._reflections.each do |key, reflection|
350+
(self.instance_reflections ||= self.class._reflections.deep_dup).each do |key, reflection|
351351
next if reflection.excluded?(self)
352352
next unless include_directive.key?(key)
353353

@@ -411,6 +411,6 @@ def associations_hash(adapter_options, options, adapter_instance)
411411

412412
protected
413413

414-
attr_accessor :instance_options
414+
attr_accessor :instance_options, :instance_reflections
415415
end
416416
end

lib/active_model/serializer/reflection.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ def include_data?(include_slice)
151151
# @yield [ActiveModel::Serializer]
152152
# @return [:nil, associated resource or resource collection]
153153
def value(serializer, include_slice)
154+
# NOTE(BF): This method isn't thread-safe because the _reflections class attribute is not thread-safe
155+
# Therefore, when we build associations from reflections, we dup the entire reflection instance.
156+
# Better solutions much appreciated!
154157
@object = serializer.object
155158
@scope = serializer.scope
156159

test/serializers/reflection_test.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,5 +423,57 @@ def test_mutating_reflection_block_is_not_thread_safe
423423
end
424424
# rubocop:enable Metrics/AbcSize
425425
end
426+
class ThreadedReflectionTest < ActiveSupport::TestCase
427+
class Post < ::Model
428+
attributes :id, :title, :body
429+
associations :comments
430+
end
431+
class Comment < ::Model
432+
attributes :id, :body
433+
associations :post
434+
end
435+
class CommentSerializer < ActiveModel::Serializer
436+
type 'comment'
437+
attributes :id, :body
438+
has_one :post
439+
end
440+
class PostSerializer < ActiveModel::Serializer
441+
type 'post'
442+
attributes :id, :title, :body
443+
has_many :comments, serializer: CommentSerializer do
444+
sleep 0.1
445+
object.comments
446+
end
447+
end
448+
449+
# per https://github.com/rails-api/active_model_serializers/issues/2270
450+
def test_concurrent_serialization
451+
post1 = Post.new(id: 1, title: 'Post 1 Title', body: 'Post 1 Body')
452+
post1.comments = [Comment.new(id: 1, body: 'Comment on Post 1', post: post1)]
453+
post2 = Post.new(id: 2, title: 'Post 2 Title', body: 'Post 2 Body')
454+
post2.comments = [Comment.new(id: 2, body: 'Comment on Post 2', post: post2)]
455+
serialized_posts = {
456+
first: Set.new,
457+
second: Set.new
458+
}
459+
t1 = Thread.new do
460+
10.times do
461+
serialized_posts[:first] << PostSerializer.new(post1, {}).to_json
462+
end
463+
end
464+
t2 = Thread.new do
465+
10.times do
466+
serialized_posts[:second] << PostSerializer.new(post2, {}).to_json
467+
end
468+
end
469+
t1.join
470+
t2.join
471+
expected_first_post_serialization = '{"id":1,"title":"Post 1 Title","body":"Post 1 Body","comments":[{"id":1,"body":"Comment on Post 1"}]}'
472+
expected_second_post_serialization = '{"id":2,"title":"Post 2 Title","body":"Post 2 Body","comments":[{"id":2,"body":"Comment on Post 2"}]}'
473+
474+
assert_equal [expected_second_post_serialization], serialized_posts[:second].to_a
475+
assert_equal [expected_first_post_serialization], serialized_posts[:first].to_a
476+
end
477+
end
426478
end
427479
end

0 commit comments

Comments
 (0)