aboutsummaryrefslogtreecommitdiffstats
path: root/activesupport
diff options
context:
space:
mode:
authorMatthew Draper <matthew@trebex.net>2016-02-07 08:24:57 +1030
committerMatthew Draper <matthew@trebex.net>2016-02-07 08:32:27 +1030
commit3e4a69e52d8c8f0335e0ddbb46fe21009b962334 (patch)
tree172481e3ea5e3ed7496ff2541a595343fbd49ae8 /activesupport
parent8526e9bed21a119266e886c3316d3fe10c9af5ce (diff)
downloadrails-3e4a69e52d8c8f0335e0ddbb46fe21009b962334.tar.gz
rails-3e4a69e52d8c8f0335e0ddbb46fe21009b962334.tar.bz2
rails-3e4a69e52d8c8f0335e0ddbb46fe21009b962334.zip
Hand off the interlock to the new thread in AC::Live
Most importantly, the original request thread must yield its share lock while waiting for the live thread to commit -- otherwise a request's base and live threads can deadlock against each other.
Diffstat (limited to 'activesupport')
-rw-r--r--activesupport/lib/active_support/concurrency/share_lock.rb48
-rw-r--r--activesupport/lib/active_support/dependencies/interlock.rb6
-rw-r--r--activesupport/test/share_lock_test.rb26
3 files changed, 65 insertions, 15 deletions
diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb
index 8e4ca272ba..2ac278a2f1 100644
--- a/activesupport/lib/active_support/concurrency/share_lock.rb
+++ b/activesupport/lib/active_support/concurrency/share_lock.rb
@@ -51,7 +51,7 @@ module ActiveSupport
if busy_for_exclusive?(purpose)
return false if no_wait
- yield_shares(purpose, compatible) do
+ yield_shares(purpose: purpose, compatible: compatible) do
@cv.wait_while { busy_for_exclusive?(purpose) }
end
end
@@ -73,10 +73,10 @@ module ActiveSupport
if @exclusive_depth == 0
@exclusive_thread = nil
- yield_shares(nil, compatible) do
- @cv.broadcast
+ yield_shares(compatible: compatible) do
@cv.wait_while { @exclusive_thread || eligible_waiters?(compatible) }
end
+ @cv.broadcast
end
end
end
@@ -127,6 +127,36 @@ module ActiveSupport
end
end
+ def yield_shares(purpose: nil, compatible: [])
+ loose_shares = previous_wait = nil
+ synchronize do
+ if loose_shares = @sharing.delete(Thread.current)
+ if previous_wait = @waiting[Thread.current]
+ purpose = nil unless purpose == previous_wait[0]
+ compatible &= previous_wait[1]
+ end
+ @waiting[Thread.current] = [purpose, compatible]
+ end
+
+ @cv.broadcast
+ end
+
+ begin
+ yield
+ ensure
+ synchronize do
+ @cv.wait_while { @exclusive_thread && @exclusive_thread != Thread.current }
+
+ if previous_wait
+ @waiting[Thread.current] = previous_wait
+ else
+ @waiting.delete Thread.current
+ end
+ @sharing[Thread.current] = loose_shares if loose_shares
+ end
+ end
+ end
+
private
# Must be called within synchronize
@@ -143,18 +173,6 @@ module ActiveSupport
def eligible_waiters?(compatible)
@waiting.any? { |t, (p, _)| compatible.include?(p) && @waiting.all? { |t2, (_, c2)| t == t2 || c2.include?(p) } }
end
-
- def yield_shares(purpose, compatible)
- loose_shares = @sharing.delete(Thread.current)
- @waiting[Thread.current] = [purpose, compatible] if loose_shares
-
- begin
- yield
- ensure
- @waiting.delete Thread.current
- @sharing[Thread.current] = loose_shares if loose_shares
- end
- end
end
end
end
diff --git a/activesupport/lib/active_support/dependencies/interlock.rb b/activesupport/lib/active_support/dependencies/interlock.rb
index b6a1b25eee..47bcecbb35 100644
--- a/activesupport/lib/active_support/dependencies/interlock.rb
+++ b/activesupport/lib/active_support/dependencies/interlock.rb
@@ -42,6 +42,12 @@ module ActiveSupport #:nodoc:
yield
end
end
+
+ def permit_concurrent_loads
+ @lock.yield_shares(compatible: [:load]) do
+ yield
+ end
+ end
end
end
end
diff --git a/activesupport/test/share_lock_test.rb b/activesupport/test/share_lock_test.rb
index 12953d99a6..68fa5bb69e 100644
--- a/activesupport/test/share_lock_test.rb
+++ b/activesupport/test/share_lock_test.rb
@@ -287,6 +287,32 @@ class ShareLockTest < ActiveSupport::TestCase
assert_threads_not_stuck threads
end
+ def test_manual_yield
+ ready = Concurrent::CyclicBarrier.new(2)
+ done = Concurrent::CyclicBarrier.new(2)
+
+ threads = [
+ Thread.new do
+ @lock.sharing do
+ ready.wait
+ @lock.exclusive(purpose: :x) {}
+ done.wait
+ end
+ end,
+
+ Thread.new do
+ @lock.sharing do
+ ready.wait
+ @lock.yield_shares(compatible: [:x]) do
+ done.wait
+ end
+ end
+ end,
+ ]
+
+ assert_threads_not_stuck threads
+ end
+
def test_in_shared_section_incompatible_non_upgrading_threads_cannot_preempt_upgrading_threads
scratch_pad = []
scratch_pad_mutex = Mutex.new