From eccc533e7490cb858ff6995eee1e8ad96b796b32 Mon Sep 17 00:00:00 2001 From: Brent Wheeldon Date: Thu, 2 Nov 2017 14:53:43 -0400 Subject: [PATCH] Prevent deadlocks with load interlock and DB lock. This fixes an issue where competing threads deadlock each other. - Thread A holds the load interlock but is blocked on getting the DB lock - Thread B holds the DB lock but is blocked on getting the load interlock (for example when there is a `Model.transaction` block that needs to autoload) This solution allows for dependency loading in other threads while a thread is waiting to acquire the DB lock. Fixes #31019 --- .../connection_adapters/abstract_adapter.rb | 3 +- .../load_interlock_aware_monitor.rb | 17 ++++++ .../load_interlock_aware_monitor_test.rb | 55 +++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb create mode 100644 activesupport/test/concurrency/load_interlock_aware_monitor_test.rb diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 84fd812392f5a..e523a97aa5d51 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -4,6 +4,7 @@ require "active_record/connection_adapters/sql_type_metadata" require "active_record/connection_adapters/abstract/schema_dumper" require "active_record/connection_adapters/abstract/schema_creation" +require "active_support/concurrency/load_interlock_aware_monitor" require "arel/collectors/bind" require "arel/collectors/sql_string" @@ -105,7 +106,7 @@ def initialize(connection, logger = nil, config = {}) # :nodoc: @schema_cache = SchemaCache.new self @quoted_column_names, @quoted_table_names = {}, {} @visitor = arel_visitor - @lock = Monitor.new + @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @prepared_statements = true diff --git a/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb b/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb new file mode 100644 index 0000000000000..a8455c00483f7 --- /dev/null +++ b/activesupport/lib/active_support/concurrency/load_interlock_aware_monitor.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require "monitor" + +module ActiveSupport + module Concurrency + # A monitor that will permit dependency loading while blocked waiting for + # the lock. + class LoadInterlockAwareMonitor < Monitor + # Enters an exclusive section, but allows dependency loading while blocked + def mon_enter + mon_try_enter || + ActiveSupport::Dependencies.interlock.permit_concurrent_loads { super } + end + end + end +end diff --git a/activesupport/test/concurrency/load_interlock_aware_monitor_test.rb b/activesupport/test/concurrency/load_interlock_aware_monitor_test.rb new file mode 100644 index 0000000000000..2d0f45ec5f380 --- /dev/null +++ b/activesupport/test/concurrency/load_interlock_aware_monitor_test.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "concurrent/atomic/count_down_latch" +require "active_support/concurrency/load_interlock_aware_monitor" + +module ActiveSupport + module Concurrency + class LoadInterlockAwareMonitorTest < ActiveSupport::TestCase + def setup + @monitor = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new + end + + def test_entering_with_no_blocking + assert @monitor.mon_enter + end + + def test_entering_with_blocking + load_interlock_latch = Concurrent::CountDownLatch.new + monitor_latch = Concurrent::CountDownLatch.new + + able_to_use_monitor = false + able_to_load = false + + thread_with_load_interlock = Thread.new do + ActiveSupport::Dependencies.interlock.running do + load_interlock_latch.count_down + monitor_latch.wait + + @monitor.synchronize do + able_to_use_monitor = true + end + end + end + + thread_with_monitor_lock = Thread.new do + @monitor.synchronize do + monitor_latch.count_down + load_interlock_latch.wait + + ActiveSupport::Dependencies.interlock.loading do + able_to_load = true + end + end + end + + thread_with_load_interlock.join + thread_with_monitor_lock.join + + assert able_to_use_monitor + assert able_to_load + end + end + end +end