Add a Monitor to ModelSchema#load_schema

[Vikrant Chaudhary, David Abdemoulaie, Matthew Draper]

activerecord/CHANGELOG.md

+*   Loading model schema from database is now thread-safe.
+
+    Fixes #28589.
+
+    *Vikrant Chaudhary*, *David Abdemoulaie*
+
 *   Add `ActiveRecord::Base#cache_version` to support recyclable cache keys via the new versioned entries
     in `ActiveSupport::Cache`. This also means that `ActiveRecord::Base#cache_key` will now return a stable key
     that does not include a timestamp any more.

activerecord/lib/active_record/model_schema.rb

+require "monitor"
+
 module ActiveRecord
   module ModelSchema
     extend ActiveSupport::Concern
@@ -152,6 +154,8 @@ module ModelSchema
       self.inheritance_column = "type"
 
       delegate :type_for_attribute, to: :class
+
+      initialize_load_schema_monitor
     end
 
     # Derives the join table name for +first_table+ and +second_table+. The
@@ -435,15 +439,27 @@ def reset_column_information
         initialize_find_by_cache
       end
 
+      protected
+
+        def initialize_load_schema_monitor
+          @load_schema_monitor = Monitor.new
+        end
+
       private
 
+        def inherited(child_class)
+          super
+          child_class.initialize_load_schema_monitor
+        end
+
         def schema_loaded?
-          defined?(@columns_hash) && @columns_hash
+          defined?(@schema_loaded) && @schema_loaded
         end
 
         def load_schema
-          unless schema_loaded?
-            load_schema!
+          return if schema_loaded?
+          @load_schema_monitor.synchronize do
+            load_schema! unless defined?(@columns_hash) && @columns_hash
           end
         end
 
@@ -457,6 +473,8 @@ def load_schema!
               user_provided_default: false
             )
           end
+
+          @schema_loaded = true
         end
 
         def reload_schema_from_cache
@@ -470,6 +488,7 @@ def reload_schema_from_cache
           @attributes_builder = nil
           @columns = nil
           @columns_hash = nil
+          @schema_loaded = false
           @attribute_names = nil
           @yaml_encoder = nil
           direct_descendants.each do |descendant|

activerecord/test/cases/serialized_attribute_test.rb

@@ -349,4 +349,32 @@ def deserialize(value)
     topic.foo
     refute topic.changed?
   end
+
+  def test_serialized_attribute_works_under_concurrent_initial_access
+    model = Topic.dup
+
+    topic = model.last
+    topic.update group: "1"
+
+    model.serialize :group, JSON
+    model.reset_column_information
+
+    # This isn't strictly necessary for the test, but a little bit of
+    # knowledge of internals allows us to make failures far more likely.
+    model.define_singleton_method(:define_attribute) do |*args|
+      Thread.pass
+      super(*args)
+    end
+
+    threads = 4.times.map do
+      Thread.new do
+        topic.reload.group
+      end
+    end
+
+    # All the threads should retrieve the value knowing it is JSON, and
+    # thus decode it. If this fails, some threads will instead see the
+    # raw string ("1"), or raise an exception.
+    assert_equal [1] * threads.size, threads.map(&:value)
+  end
 end