Skip to content

Commit 97605a0

Browse files
committed
Fix potential deadlock in nested locking
1 parent 23b9659 commit 97605a0

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

lib/pg_ha_migrations/safe_statements.rb

+6
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,12 @@ def safely_acquire_lock_for_table(*tables, mode: :access_exclusive, &block)
564564

565565
until successfully_acquired_lock
566566
loop do
567+
# If in a nested context and all of the above checks have passed,
568+
# we have already acquired the lock so this check is unnecessary.
569+
# In fact, it could actually cause a deadlock if a blocking query
570+
# was executed shortly after the initial lock acquisition.
571+
break if nested_target_tables
572+
567573
blocking_transactions = PgHaMigrations::BlockingDatabaseTransactions.find_blocking_transactions("#{PgHaMigrations::LOCK_TIMEOUT_SECONDS} seconds")
568574

569575
# Locking a partitioned table will also lock child tables (including sub-partitions),

spec/safe_statements_spec.rb

+39
Original file line numberDiff line numberDiff line change
@@ -4781,6 +4781,45 @@ def up
47814781
expect(locks_for_table(table_name, connection: alternate_connection)).to be_empty
47824782
end
47834783

4784+
it "skips blocking query check for nested lock acquisition" do
4785+
stub_const("PgHaMigrations::LOCK_TIMEOUT_SECONDS", 1)
4786+
4787+
query_thread = nil
4788+
4789+
expect(PgHaMigrations::BlockingDatabaseTransactions).to receive(:find_blocking_transactions)
4790+
.once
4791+
.and_call_original
4792+
4793+
begin
4794+
migration.safely_acquire_lock_for_table(table_name) do
4795+
query_thread = Thread.new { alternate_connection.execute("SELECT * FROM bogus_table") }
4796+
4797+
sleep 2
4798+
4799+
migration.safely_acquire_lock_for_table(table_name) do
4800+
expect(locks_for_table(table_name, connection: alternate_connection_2)).to contain_exactly(
4801+
having_attributes(
4802+
table: "bogus_table",
4803+
lock_type: "AccessExclusiveLock",
4804+
granted: true,
4805+
pid: kind_of(Integer),
4806+
),
4807+
having_attributes(
4808+
table: "bogus_table",
4809+
lock_type: "AccessShareLock",
4810+
granted: false,
4811+
pid: kind_of(Integer),
4812+
),
4813+
)
4814+
end
4815+
end
4816+
ensure
4817+
query_thread.join if query_thread
4818+
end
4819+
4820+
expect(locks_for_table(table_name, connection: alternate_connection)).to be_empty
4821+
end
4822+
47844823
it "does not allow re-entrancy when lock escalation detected" do
47854824
expect do
47864825
migration.safely_acquire_lock_for_table(table_name, mode: :share) do

0 commit comments

Comments
 (0)