From 4c54b2a9a012296709de5283eada03470d581dc9 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
Date: Tue, 21 Jul 2015 11:47:16 +0930
Subject: Adjust expectations around purpose/compatibility options

---
 .../lib/active_support/concurrency/share_lock.rb   |  2 +-
 activesupport/test/share_lock_test.rb              | 43 ++++++++++++++++++----
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb
index 48edcfdaa5..39ae9bfb79 100644
--- a/activesupport/lib/active_support/concurrency/share_lock.rb
+++ b/activesupport/lib/active_support/concurrency/share_lock.rb
@@ -132,7 +132,7 @@ module ActiveSupport
       # Must be called within synchronize
       def busy?(purpose)
         (@exclusive_thread && @exclusive_thread != Thread.current) ||
-          (purpose && @waiting.any? { |k, v| k != Thread.current && !v.include?(purpose) }) ||
+          @waiting.any? { |k, v| k != Thread.current && !v.include?(purpose) } ||
           @sharing.size > (@sharing[Thread.current] > 0 ? 1 : 0)
       end
     end
diff --git a/activesupport/test/share_lock_test.rb b/activesupport/test/share_lock_test.rb
index 8ca2a46a6c..efd840be79 100644
--- a/activesupport/test/share_lock_test.rb
+++ b/activesupport/test/share_lock_test.rb
@@ -88,33 +88,60 @@ class ShareLockTest < ActiveSupport::TestCase
           conflicting_exclusive_threads = [
             Thread.new do
               @lock.send(use_upgrading ? :sharing : :tap) do
-                @lock.exclusive(purpose: :load, compatible: [:load]) {}
+                @lock.exclusive(purpose: :red, compatible: [:green, :purple]) {}
               end
             end,
             Thread.new do
               @lock.send(use_upgrading ? :sharing : :tap) do
-                @lock.exclusive(purpose: :unload, compatible: [:unload]) {}
+                @lock.exclusive(purpose: :blue, compatible: [:green]) {}
               end
             end
           ]
 
           assert_threads_stuck conflicting_exclusive_threads # wait for threads to get into their respective `exclusive {}` blocks
-          sharing_thread_release_latch.count_down
-          assert_threads_stuck conflicting_exclusive_threads # assert they are stuck
 
+          # This thread will be stuck as long as any other thread is in
+          # a sharing block. While it's blocked, it holds no lock, so it
+          # doesn't interfere with any other attempts.
           no_purpose_thread = Thread.new do
             @lock.exclusive {}
           end
-          assert_threads_not_stuck no_purpose_thread # no purpose thread is able to squeak through
+          assert_threads_stuck no_purpose_thread
 
+          # This thread is compatible with both of the "primary"
+          # attempts above. It's initially stuck on the outer share
+          # lock, but as soon as that's released, it can run --
+          # regardless of whether those threads hold share locks.
           compatible_thread = Thread.new do
-            @lock.exclusive(purpose: :load, compatible: [:load, :unload])
+            @lock.exclusive(purpose: :green, compatible: []) {}
           end
+          assert_threads_stuck compatible_thread
+
+          assert_threads_stuck conflicting_exclusive_threads
+
+          sharing_thread_release_latch.count_down
 
-          assert_threads_not_stuck compatible_thread # compatible thread is able to squeak through
-          assert_threads_stuck conflicting_exclusive_threads # assert other threads are still stuck
+          assert_threads_not_stuck compatible_thread # compatible thread is now able to squeak through
+
+          if use_upgrading
+            # The "primary" threads both each hold a share lock, and are
+            # mutually incompatible; they're still stuck.
+            assert_threads_stuck conflicting_exclusive_threads
+
+            # The thread without a specified purpose is also stuck; it's
+            # not compatible with anything.
+            assert_threads_stuck no_purpose_thread
+          else
+            # As the primaries didn't hold a share lock, as soon as the
+            # outer one was released, all the exclusive locks are free
+            # to be acquired in turn.
+
+            assert_threads_not_stuck conflicting_exclusive_threads
+            assert_threads_not_stuck no_purpose_thread
+          end
         ensure
           conflicting_exclusive_threads.each(&:kill)
+          no_purpose_thread.kill
         end
       end
     end
-- 
cgit v1.2.3