We need stricter locking before we can unload

Specifically, the "loose upgrades" behaviour that allows us to obtain an
exclusive right to load things while other requests are in progress (but
waiting on the exclusive lock for themselves) prevents us from treating
load & unload interchangeably: new things appearing is fine, but they do
*not* expect previously-present constants to vanish.

We can still use loose upgrades for unloading -- once someone has
decided to unload, they don't really care if someone else gets there
first -- it just needs to be tracked separately.

1 files changed, 29 insertions, 27 deletions

diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb
index f1c6230084..39ae9bfb79 100644
--- a/activesupport/lib/active_support/concurrency/share_lock.rb
+++ b/activesupport/lib/active_support/concurrency/share_lock.rb
@@ -9,7 +9,7 @@ module ActiveSupport
     #--
     # Note that a pending Exclusive lock attempt does not block incoming
     # Share requests (i.e., we are "read-preferring"). That seems
-    # consistent with the behavior of +loose_upgrades+, but may be the
+    # consistent with the behavior of "loose" upgrades, but may be the
     # wrong choice otherwise: it nominally reduces the possibility of
     # deadlock by risking starvation instead.
     class ShareLock
@@ -20,47 +20,46 @@ module ActiveSupport
       # 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
-
+      def initialize
         super()
 
         @cv = new_cond
 
         @sharing = Hash.new(0)
+        @waiting = {}
         @exclusive_thread = nil
         @exclusive_depth = 0
       end
 
-      # Returns false if +no_wait+ is specified and the lock is not
+      # Returns false if +no_wait+ is set and the lock is not
       # immediately available. Otherwise, returns true after the lock
       # has been acquired.
-      def start_exclusive(no_wait=false)
+      #
+      # +purpose+ and +compatible+ work together; while this thread is
+      # waiting for the exclusive lock, it will yield its share (if any)
+      # to any other attempt whose +purpose+ appears in this attempt's
+      # +compatible+ list. This allows a "loose" upgrade, which, being
+      # less strict, 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. With
+      # +purpose+ matching, it is possible to yield only to other
+      # threads whose activity will not interfere.
+      def start_exclusive(purpose: nil, compatible: [], no_wait: false)
         synchronize do
           unless @exclusive_thread == Thread.current
-            return false if no_wait && busy?
+            if busy?(purpose)
+              return false if no_wait
 
-            loose_shares = nil
-            if @loose_upgrades
               loose_shares = @sharing.delete(Thread.current)
-            end
+              @waiting[Thread.current] = compatible if loose_shares
 
-            @cv.wait_while { busy? } if busy?
+              @cv.wait_while { busy?(purpose) }
 
+              @waiting.delete Thread.current
+              @sharing[Thread.current] = loose_shares if loose_shares
+            end
             @exclusive_thread = Thread.current
-            @sharing[Thread.current] = loose_shares if loose_shares
           end
           @exclusive_depth += 1
 
@@ -106,8 +105,10 @@ module ActiveSupport
       # +no_wait+ is set and the lock is not immediately available,
       # returns +nil+ without yielding. Otherwise, returns the result of
       # the block.
-      def exclusive(no_wait=false)
-        if start_exclusive(no_wait)
+      #
+      # See +start_exclusive+ for other options.
+      def exclusive(purpose: nil, compatible: [], no_wait: false)
+        if start_exclusive(purpose: purpose, compatible: compatible, no_wait: no_wait)
           begin
             yield
           ensure
@@ -129,8 +130,9 @@ module ActiveSupport
       private
 
       # Must be called within synchronize
-      def busy?
+      def busy?(purpose)
         (@exclusive_thread && @exclusive_thread != Thread.current) ||
+          @waiting.any? { |k, v| k != Thread.current && !v.include?(purpose) } ||
           @sharing.size > (@sharing[Thread.current] > 0 ? 1 : 0)
       end
     end