Fix multi-threaded issue for AcceptanceValidator

rails · Nov 3, 2019 · 98754de · 98754de
1 parent 9bfb73a
commit 98754de
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 7 deletions.
diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb
@@ -30,6 +30,7 @@ def initialize(attributes)
           end
 
           def included(klass)
+            @lock = Mutex.new
             mod = self
 
             define_method(:respond_to_missing?) do |method_name, include_private = false|
@@ -53,14 +54,20 @@ def matches?(method_name)
           end
 
           def define_on(klass)
-            remove_method :respond_to_missing?
-            remove_method :method_missing
+            @lock&.synchronize do
+              return unless @lock
 
-            attr_readers = attributes.reject { |name| klass.attribute_method?(name) }
-            attr_writers = attributes.reject { |name| klass.attribute_method?("#{name}=") }
+              attr_readers = attributes.reject { |name| klass.attribute_method?(name) }
+              attr_writers = attributes.reject { |name| klass.attribute_method?("#{name}=") }
 
-            attr_reader(*attr_readers)
-            attr_writer(*attr_writers)
+              attr_reader(*attr_readers)
+              attr_writer(*attr_writers)
+
+              remove_method :respond_to_missing?
+              remove_method :method_missing
+
+              @lock = nil
+            end
           end
 
           def ==(other)

diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb
@@ -122,7 +122,13 @@ def test_lazy_attributes_respond_to?
     klass = define_test_class(Topic)
     klass.validates_acceptance_of(:terms_of_service)
     topic = klass.new
-    assert topic.respond_to?(:terms_of_service)
+    threads = []
+    2.times do
+      threads << Thread.new do
+        assert topic.respond_to?(:terms_of_service)
+      end
+    end
+    threads.each(&:join)
   end
 
   private