Soften the lock requirements when eager_load is disabled

We don't need to fully disable concurrent requests: just ensure that
loads are performed in isolation.

3 files changed, 200 insertions, 34 deletions

diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb
new file mode 100644
index 0000000000..e03f2cfc7a
--- /dev/null
+++ b/activesupport/lib/active_support/concurrency/share_lock.rb
@@ -0,0 +1,118 @@
+require 'thread'
+require 'monitor'
+
+module ActiveSupport
+  module Concurrency
+    class ShareLock
+      include MonitorMixin
+
+      # We track Thread objects, instead of just using counters, because
+      # we need exclusive locks to be reentrant, and we need to be able
+      # to upgrade share locks to exclusive.
+
+
+      # If +loose_upgrades+ is false (the default), then a thread that
+      # is waiting on an Exclusive lock will continue to hold any Share
+      # lock that it has already established. This is safer, but can
+      # lead to deadlock.
+      #
+      # If +loose_upgrades+ is true, a thread waiting on an Exclusive
+      # lock will temporarily relinquish its Share lock. Being less
+      # strict, this behavior prevents some classes of deadlocks. For
+      # many resources, loose upgrades are sufficient: if a thread is
+      # awaiting a lock, it is not running any other code.
+      attr_reader :loose_upgrades
+
+      def initialize(loose_upgrades = false)
+        @loose_upgrades = loose_upgrades
+
+        super()
+
+        @cv = new_cond
+
+        @sharing = Hash.new(0)
+        @exclusive_thread = nil
+        @exclusive_depth = 0
+      end
+
+      def start_exclusive(no_wait=false)
+        synchronize do
+          unless @exclusive_thread == Thread.current
+            return false if no_wait && busy?
+
+            loose_shares = nil
+            if @loose_upgrades
+              loose_shares = @sharing.delete(Thread.current)
+            end
+
+            @cv.wait_while { busy? } if busy?
+
+            @exclusive_thread = Thread.current
+            @sharing[Thread.current] = loose_shares if loose_shares
+          end
+          @exclusive_depth += 1
+
+          true
+        end
+      end
+
+      def stop_exclusive
+        synchronize do
+          raise "invalid unlock" if @exclusive_thread != Thread.current
+
+          @exclusive_depth -= 1
+          if @exclusive_depth == 0
+            @exclusive_thread = nil
+            @cv.broadcast
+          end
+        end
+      end
+
+      def start_sharing
+        synchronize do
+          if @exclusive_thread && @exclusive_thread != Thread.current
+            @cv.wait_while { @exclusive_thread }
+          end
+          @sharing[Thread.current] += 1
+        end
+      end
+
+      def stop_sharing
+        synchronize do
+          if @sharing[Thread.current] > 1
+            @sharing[Thread.current] -= 1
+          else
+            @sharing.delete Thread.current
+            @cv.broadcast
+          end
+        end
+      end
+
+      def exclusive(no_wait=false)
+        if start_exclusive(no_wait)
+          begin
+            yield
+          ensure
+            stop_exclusive
+          end
+        end
+      end
+
+      def sharing
+        start_sharing
+        begin
+          yield
+        ensure
+          stop_sharing
+        end
+      end
+
+      private
+
+      def busy?
+        (@exclusive_thread && @exclusive_thread != Thread.current) ||
+          @sharing.size > (@sharing[Thread.current] > 0 ? 1 : 0)
+      end
+    end
+  end
+end
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 664cc15a29..2a0e06495f 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -12,12 +12,16 @@ require 'active_support/core_ext/kernel/reporting'
 require 'active_support/core_ext/load_error'
 require 'active_support/core_ext/name_error'
 require 'active_support/core_ext/string/starts_ends_with'
+require "active_support/dependencies/interlock"
 require 'active_support/inflector'
 
 module ActiveSupport #:nodoc:
   module Dependencies #:nodoc:
     extend self
 
+    mattr_accessor :interlock
+    self.interlock = Interlock.new
+
     # Should we turn on Ruby warnings on the first load of dependent files?
     mattr_accessor :warnings_on_first_load
     self.warnings_on_first_load = false
@@ -234,10 +238,12 @@ module ActiveSupport #:nodoc:
       end
 
       def load_dependency(file)
-        if Dependencies.load? && ActiveSupport::Dependencies.constant_watch_stack.watching?
-          Dependencies.new_constants_in(Object) { yield }
-        else
-          yield
+        Dependencies.interlock.loading do
+          if Dependencies.load? && ActiveSupport::Dependencies.constant_watch_stack.watching?
+            Dependencies.new_constants_in(Object) { yield }
+          else
+            yield
+          end
         end
       rescue Exception => exception  # errors from loading file
         exception.blame_file! file if exception.respond_to? :blame_file!
@@ -325,9 +331,11 @@ module ActiveSupport #:nodoc:
 
     def clear
       log_call
-      loaded.clear
-      loading.clear
-      remove_unloadable_constants!
+      Dependencies.interlock.loading do
+        loaded.clear
+        loading.clear
+        remove_unloadable_constants!
+      end
     end
 
     def require_or_load(file_name, const_path = nil)
@@ -336,39 +344,44 @@ module ActiveSupport #:nodoc:
       expanded = File.expand_path(file_name)
       return if loaded.include?(expanded)
 
-      # Record that we've seen this file *before* loading it to avoid an
-      # infinite loop with mutual dependencies.
-      loaded << expanded
-      loading << expanded
-
-      begin
-        if load?
-          log "loading #{file_name}"
+      Dependencies.interlock.loading do
+        # Maybe it got loaded while we were waiting for our lock:
+        return if loaded.include?(expanded)
 
-          # Enable warnings if this file has not been loaded before and
-          # warnings_on_first_load is set.
-          load_args = ["#{file_name}.rb"]
-          load_args << const_path unless const_path.nil?
+        # Record that we've seen this file *before* loading it to avoid an
+        # infinite loop with mutual dependencies.
+        loaded << expanded
+        loading << expanded
 
-          if !warnings_on_first_load or history.include?(expanded)
-            result = load_file(*load_args)
+        begin
+          if load?
+            log "loading #{file_name}"
+
+            # Enable warnings if this file has not been loaded before and
+            # warnings_on_first_load is set.
+            load_args = ["#{file_name}.rb"]
+            load_args << const_path unless const_path.nil?
+
+            if !warnings_on_first_load or history.include?(expanded)
+              result = load_file(*load_args)
+            else
+              enable_warnings { result = load_file(*load_args) }
+            end
           else
-            enable_warnings { result = load_file(*load_args) }
+            log "requiring #{file_name}"
+            result = require file_name
           end
-        else
-          log "requiring #{file_name}"
-          result = require file_name
+        rescue Exception
+          loaded.delete expanded
+          raise
+        ensure
+          loading.pop
         end
-      rescue Exception
-        loaded.delete expanded
-        raise
-      ensure
-        loading.pop
-      end
 
-      # Record history *after* loading so first load gets warnings.
-      history << expanded
-      result
+        # Record history *after* loading so first load gets warnings.
+        history << expanded
+        result
+      end
     end
 
     # Is the provided constant path defined?
diff --git a/activesupport/lib/active_support/dependencies/interlock.rb b/activesupport/lib/active_support/dependencies/interlock.rb
new file mode 100644
index 0000000000..c3601bac13
--- /dev/null
+++ b/activesupport/lib/active_support/dependencies/interlock.rb
@@ -0,0 +1,35 @@
+require 'active_support/concurrency/share_lock'
+
+module ActiveSupport
+  module Dependencies #:nodoc:
+    class Interlock
+      def initialize
+        @lock = ActiveSupport::Concurrency::ShareLock.new(true)
+      end
+
+      def loading
+        @lock.exclusive do
+          yield
+        end
+      end
+
+      def start_running
+        @lock.start_sharing
+      end
+
+      def done_running
+        @lock.stop_sharing
+      end
+
+      def running
+        @lock.sharing do
+          yield
+        end
+      end
+
+      # Match the Mutex API, so we can be used by Rack::Lock
+      alias :lock :start_running
+      alias :unlock :done_running
+    end
+  end
+end