diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b3a3ec5..f2e824d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Fix rare race condition causing `NameError` (missing `@dehydrated_*` ivar) when hydrating recursively + embedded associations by synchronizing hydration to be thread-safe. + ## 1.6.3 - Split the `with_deferred_parent_expiration` and `with_deferred_parent_expiration`. (#578) diff --git a/lib/identity_cache/cached/recursive/association.rb b/lib/identity_cache/cached/recursive/association.rb index 6b573b2e..5bee1ff0 100644 --- a/lib/identity_cache/cached/recursive/association.rb +++ b/lib/identity_cache/cached/recursive/association.rb @@ -7,6 +7,7 @@ class Association < Cached::Association # :nodoc: def initialize(name, reflection:) super @dehydrated_variable_name = :"@dehydrated_#{name}" + @hydration_mutex = Mutex.new end attr_reader :dehydrated_variable_name @@ -29,10 +30,7 @@ def read(record) if record.instance_variable_defined?(records_variable_name) record.instance_variable_get(records_variable_name) elsif record.instance_variable_defined?(dehydrated_variable_name) - dehydrated_target = record.instance_variable_get(dehydrated_variable_name) - association_target = hydrate_association_target(assoc.klass, dehydrated_target) - record.remove_instance_variable(dehydrated_variable_name) - set_with_inverse(record, association_target) + hydrate_safely(record, assoc) else assoc.load_target end @@ -76,6 +74,21 @@ def embedded_recursively? private + def hydrate_safely(record, assoc) + @hydration_mutex.synchronize do + if record.instance_variable_defined?(records_variable_name) + return record.instance_variable_get(records_variable_name) + end + + dehydrated_target = record.instance_variable_get(dehydrated_variable_name) + association_target = hydrate_association_target(assoc.klass, dehydrated_target) + if record.instance_variable_defined?(dehydrated_variable_name) + record.remove_instance_variable(dehydrated_variable_name) + end + set_with_inverse(record, association_target) + end + end + def set_inverse(record, association_target) return if association_target.nil? diff --git a/test/cached/recursive/has_many_test.rb b/test/cached/recursive/has_many_test.rb index 349f5868..3e3ddb3d 100644 --- a/test/cached/recursive/has_many_test.rb +++ b/test/cached/recursive/has_many_test.rb @@ -45,6 +45,33 @@ def test_embedded_recursively def test_embedded_by_reference refute_predicate(has_many, :embedded_by_reference?) end + + def test_thread_safety_hydration + # Simulate race condition by setting up a dehydrated record + record = AssociatedRecord.new + dehydrated_data = [{ "id" => 1, "name" => "Test" }] # Mock dehydrated data + record.instance_variable_set(has_many.dehydrated_variable_name, dehydrated_data) + + # Simulate multiple threads trying to hydrate concurrently + threads = [] + errors = [] + + 10.times do + threads << Thread.new do + begin + # Call the cached accessor which triggers hydration + record.fetch_deeply_associated_records + rescue => e + errors << e + end + end + end + + threads.each(&:join) + + # Assert no NameError occurred (which would indicate the race condition) + assert_empty(errors, "Race condition detected: #{errors.map(&:message).join(', ')}") + end end end end