aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorMatthew Draper <matthew@trebex.net>2017-05-26 08:06:16 +0930
committerGitHub <noreply@github.com>2017-05-26 08:06:16 +0930
commitb2556fd8a4bc42beabf25dd199fd454963ccd754 (patch)
treea0a17622bb421f4d37f45952fb935e0149803b2f /activerecord
parentaee5ef3d67664e964f5adc2ead6e1fe64e5b13dc (diff)
parent6a5a814eaa0d4f98d6151df5bd96cc8ec1ae5675 (diff)
downloadrails-b2556fd8a4bc42beabf25dd199fd454963ccd754.tar.gz
rails-b2556fd8a4bc42beabf25dd199fd454963ccd754.tar.bz2
rails-b2556fd8a4bc42beabf25dd199fd454963ccd754.zip
Merge pull request #29216 from matthewd/threadsafe-load-schema
Add a Monitor to ModelSchema#load_schema
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md6
-rw-r--r--activerecord/lib/active_record/model_schema.rb25
-rw-r--r--activerecord/test/cases/serialized_attribute_test.rb28
3 files changed, 56 insertions, 3 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 907dd894cd..d17bbf80ca 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+* 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.
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
index 54216caaaf..5095cdfe9f 100644
--- a/activerecord/lib/active_record/model_schema.rb
+++ b/activerecord/lib/active_record/model_schema.rb
@@ -1,3 +1,5 @@
+require "monitor"
+
module ActiveRecord
module ModelSchema
extend ActiveSupport::Concern
@@ -152,6 +154,8 @@ module ActiveRecord
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 @@ module ActiveRecord
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 @@ module ActiveRecord
user_provided_default: false
)
end
+
+ @schema_loaded = true
end
def reload_schema_from_cache
@@ -470,6 +488,7 @@ module ActiveRecord
@attributes_builder = nil
@columns = nil
@columns_hash = nil
+ @schema_loaded = false
@attribute_names = nil
@yaml_encoder = nil
direct_descendants.each do |descendant|
diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb
index 673392b4c4..e1bdaab5cf 100644
--- a/activerecord/test/cases/serialized_attribute_test.rb
+++ b/activerecord/test/cases/serialized_attribute_test.rb
@@ -349,4 +349,32 @@ class SerializedAttributeTest < ActiveRecord::TestCase
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