Skip to content

Commit

Permalink
Fix default scope thread safety. Thanks @thedarkone for reporting.
Browse files Browse the repository at this point in the history
  • Loading branch information
jonleighton committed Aug 13, 2011
1 parent 291072a commit 24f902b
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 15 deletions.
41 changes: 26 additions & 15 deletions activerecord/lib/active_record/base.rb
Expand Up @@ -1206,6 +1206,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.
#
Expand Down Expand Up @@ -1259,24 +1267,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
Expand Down
18 changes: 18 additions & 0 deletions activerecord/test/cases/relation_scoping_test.rb
Expand Up @@ -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
9 changes: 9 additions & 0 deletions activerecord/test/models/developer.rb
Expand Up @@ -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

0 comments on commit 24f902b

Please sign in to comment.