Merge pull request rails#53890 from byroot/fix-lazy-attr-method-definition-thread-safety

Fix lazy attribute method definitiont to be thread safe

Author: byroot

activerecord/lib/active_record/attribute_methods.rb

@@ -116,10 +116,11 @@ def define_attribute_methods # :nodoc:
             alias_attribute :id_value, :id if _has_attribute?("id")
           end
 
-          @attribute_methods_generated = true
-
           generate_alias_attributes
+
+          @attribute_methods_generated = true
         end
+
         true
       end
 
@@ -472,23 +473,27 @@ def respond_to_missing?(name, include_private = false)
       end
 
       def method_missing(name, ...)
-        unless self.class.attribute_methods_generated?
-          if self.class.method_defined?(name)
-            # The method is explicitly defined in the model, but calls a generated
-            # method with super. So we must resume the call chain at the right step.
-            last_method = method(name)
-            last_method = last_method.super_method while last_method.super_method
-            self.class.define_attribute_methods
-            if last_method.super_method
-              return last_method.super_method.call(...)
-            end
-          elsif self.class.define_attribute_methods
-            # Some attribute methods weren't generated yet, we retry the call
-            return public_send(name, ...)
-          end
+        # We can't know whether some method was defined or not because
+        # multiple thread might be concurrently be in this code path.
+        # So the first one would define the methods and the others would
+        # appear to already have them.
+        self.class.define_attribute_methods
+
+        # So in all cases we must behave as if the method was just defined.
+        method = begin
+          self.class.public_instance_method(name)
+        rescue NameError
+          nil
         end
 
-        super
+        # The method might be explicitly defined in the model, but call a generated
+        # method with super. So we must resume the call chain at the right step.
+        method = method.super_method while method && !method.owner.is_a?(GeneratedAttributeMethods)
+        if method
+          method.bind_call(self, ...)
+        else
+          super
+        end
       end
 
       def attribute_method?(attr_name)

activerecord/test/cases/attribute_methods_test.rb

@@ -1143,6 +1143,54 @@ def name
     assert_nothing_raised { klass.define_attribute_method("bar") }
   end
 
+  test "#method_missing define methods on the fly in a thread safe way" do
+    topic_class = Class.new(ActiveRecord::Base) do
+      self.table_name = "topics"
+    end
+
+    topic = topic_class.new(title: "New topic")
+    topic_class.undefine_attribute_methods
+    def topic.method_missing(...)
+      sleep 0.1 # required to cause a race condition
+      super
+    end
+
+    threads = 5.times.map do
+      Thread.new do
+        assert_equal "New topic", topic.title
+      end
+    end
+    threads.each(&:join)
+  ensure
+    threads&.each(&:kill)
+  end
+
+  test "#method_missing define methods on the fly in a thread safe way, even when decorated" do
+    topic_class = Class.new(ActiveRecord::Base) do
+      self.table_name = "topics"
+
+      def title
+        "title:#{super}"
+      end
+    end
+
+    topic = topic_class.new(title: "New topic")
+    topic_class.undefine_attribute_methods
+    def topic.method_missing(...)
+      sleep 0.1 # required to cause a race condition
+      super
+    end
+
+    threads = 5.times.map do
+      Thread.new do
+        assert_equal "title:New topic", topic.title
+      end
+    end
+    threads.each(&:join)
+  ensure
+    threads&.each(&:kill)
+  end
+
   test "read_attribute with nil should not asplode" do
     assert_nil Topic.new.read_attribute(nil)
   end