Skip to content

Commit f562860

Browse files
committed
Disconnect if configure_connection failed
Fix: rails#51780 There are a number of scenarios in which one of the queries performed as part of `configure_connection` fails, and leave the `Adapter` in an half initialized state. Up to Rails 7.0, this could likely happen during a call to `reconnect!`, so relatively rare. For the more classic initial connection, the `Adapter` instance would only be added to the pool on a successful connect. But in 7.1 I made the connection fully lazy, meaning the `Adapter` instance is now added to the pool before attempting to connect. This made this bug much more likely. The solution is to disconnect if `configure_connection` raises an error. This way on a retry we'll reconnect and reconfigure from scratch.
1 parent aa2bfad commit f562860

File tree

2 files changed

+31
-3
lines changed

2 files changed

+31
-3
lines changed

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ def reconnect!(restore_transactions: false)
677677

678678
reset_transaction(restore: restore_transactions) do
679679
clear_cache!(new_connection: true)
680-
configure_connection
680+
attempt_configure_connection
681681
end
682682
rescue => original_exception
683683
translated_exception = translate_exception_class(original_exception, nil, nil)
@@ -730,7 +730,7 @@ def discard!
730730
def reset!
731731
clear_cache!(new_connection: true)
732732
reset_transaction
733-
configure_connection
733+
attempt_configure_connection
734734
end
735735

736736
# Removes the connection from the pool and disconnect it.
@@ -766,7 +766,7 @@ def verify!
766766
if @unconfigured_connection
767767
@raw_connection = @unconfigured_connection
768768
@unconfigured_connection = nil
769-
configure_connection
769+
attempt_configure_connection
770770
@last_activity = Process.clock_gettime(Process::CLOCK_MONOTONIC)
771771
@verified = true
772772
return
@@ -1219,6 +1219,13 @@ def configure_connection
12191219
check_version
12201220
end
12211221

1222+
def attempt_configure_connection
1223+
configure_connection
1224+
rescue
1225+
disconnect!
1226+
raise
1227+
end
1228+
12221229
def default_prepared_statements
12231230
true
12241231
end

activerecord/test/cases/adapter_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,27 @@ def teardown
873873
@connection.execute("SELECT 1", allow_retry: true)
874874
end
875875

876+
test "disconnect and recover on #configure_connection failure" do
877+
connection = ActiveRecord::Base.connection_pool.send(:new_connection)
878+
879+
failures = [ActiveRecord::ConnectionFailed.new("Oops"), ActiveRecord::ConnectionFailed.new("Oops 2")]
880+
connection.singleton_class.define_method(:configure_connection) do
881+
if error = failures.pop
882+
raise error
883+
else
884+
super()
885+
end
886+
end
887+
assert_raises ActiveRecord::ConnectionFailed do
888+
connection.exec_query("SELECT 1")
889+
end
890+
891+
assert_equal [[1]], connection.exec_query("SELECT 1").rows
892+
assert_empty failures
893+
ensure
894+
connection&.disconnect!
895+
end
896+
876897
private
877898
def raw_transaction_open?(connection)
878899
case connection.adapter_name

0 commit comments

Comments
 (0)