Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix default scope thread safety. Thanks @thedarkone for reporting.

  • Loading branch information...
commit af96a9179308f52e4d665e474b795a01a9a6641f 1 parent 2db0455
@jonleighton jonleighton authored
View
41 activerecord/lib/active_record/base.rb
@@ -1220,6 +1220,14 @@ def current_scope=(scope) #:nodoc:
Thread.current["#{self}_current_scope"] = scope
end
+ def ignore_default_scope? #:nodoc:
+ Thread.current["#{self}_ignore_default_scope"]
+ end
+
+ def ignore_default_scope=(ignore) #:nodoc:
+ Thread.current["#{self}_ignore_default_scope"] = ignore
+ end
+
# Use this macro in your model to set a default scope for all operations on
# the model.
#
@@ -1273,24 +1281,27 @@ def default_scope(scope = {})
# The @ignore_default_scope flag is used to prevent an infinite recursion situation where
# a default scope references a scope which has a default scope which references a scope...
def build_default_scope #:nodoc:
- return if defined?(@ignore_default_scope) && @ignore_default_scope
- @ignore_default_scope = true
-
- if method(:default_scope).owner != Base.singleton_class
- default_scope
- elsif default_scopes.any?
- default_scopes.inject(relation) do |default_scope, scope|
- if scope.is_a?(Hash)
- default_scope.apply_finder_options(scope)
- elsif !scope.is_a?(Relation) && scope.respond_to?(:call)
- default_scope.merge(scope.call)
- else
- default_scope.merge(scope)
+ return if ignore_default_scope?
+
+ begin
+ self.ignore_default_scope = true
+
+ if method(:default_scope).owner != Base.singleton_class
+ default_scope
+ elsif default_scopes.any?
+ default_scopes.inject(relation) do |default_scope, scope|
+ if scope.is_a?(Hash)
+ default_scope.apply_finder_options(scope)
+ elsif !scope.is_a?(Relation) && scope.respond_to?(:call)
+ default_scope.merge(scope.call)
+ else
+ default_scope.merge(scope)
+ end
end
end
+ ensure
+ self.ignore_default_scope = false
end
- ensure
- @ignore_default_scope = false
end
# Returns the class type of the record using the current module as a prefix. So descendants of
View
18 activerecord/test/cases/relation_scoping_test.rb
@@ -524,4 +524,22 @@ def test_default_scope_include_with_count
assert_equal 1, DeveloperWithIncludes.where(:audit_logs => { :message => 'foo' }).count
end
+
+ def test_default_scope_is_threadsafe
+ if in_memory_db?
+ skip "in memory db can't share a db between threads"
+ end
+
+ threads = []
+ assert_not_equal 1, ThreadsafeDeveloper.unscoped.count
+
+ threads << Thread.new do
+ Thread.current[:long_default_scope] = true
+ assert_equal 1, ThreadsafeDeveloper.all.count
+ end
+ threads << Thread.new do
+ assert_equal 1, ThreadsafeDeveloper.all.count
+ end
+ threads.each(&:join)
+ end
end
View
9 activerecord/test/models/developer.rb
@@ -227,3 +227,12 @@ class EagerDeveloperWithCallableDefaultScope < ActiveRecord::Base
default_scope OpenStruct.new(:call => includes(:projects))
end
+
+class ThreadsafeDeveloper < ActiveRecord::Base
+ self.table_name = 'developers'
+
+ def self.default_scope
+ sleep 0.05 if Thread.current[:long_default_scope]
+ limit(1)
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.