Skip to content

Commit 8533003

Browse files
committed
ActiveRecord::TestCase: reap all leaked connection on teardown
Some tests do leak connections, which cause some other tests to stall for a very long time when they call `clear_all_connections`. Each leaked connection takes 5 seconds to be cleared because of the checkout timeout. So on teardown to inspect all pools to make sure nothing was leaked and if some connections were leaked we cleanup the state.
1 parent 9aeb1de commit 8533003

File tree

3 files changed

+65
-6
lines changed

3 files changed

+65
-6
lines changed

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,15 @@ def release_connection(owner_thread = ActiveSupport::IsolatedExecutionState.cont
252252
# connection will be properly returned to the pool by the code that checked
253253
# it out.
254254
def with_connection
255-
unless conn = @thread_cached_conns[ActiveSupport::IsolatedExecutionState.context]
256-
conn = connection
257-
fresh_connection = true
255+
if conn = @thread_cached_conns[ActiveSupport::IsolatedExecutionState.context]
256+
yield conn
257+
else
258+
begin
259+
yield connection
260+
ensure
261+
release_connection
262+
end
258263
end
259-
yield conn
260-
ensure
261-
release_connection if fresh_connection
262264
end
263265

264266
# Returns true if a connection has already been opened.
@@ -573,6 +575,8 @@ def with_exclusively_acquired_all_connections(raise_on_acquisition_timeout = tru
573575

574576
def attempt_to_checkout_all_existing_connections(raise_on_acquisition_timeout = true)
575577
collected_conns = synchronize do
578+
reap # No need to wait for dead owners
579+
576580
# account for our own connections
577581
@connections.select { |conn| conn.owner == ActiveSupport::IsolatedExecutionState.context }
578582
end

activerecord/test/cases/connection_pool_test.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,11 @@ def join(timeout = nil)
978978
def terminate
979979
nil
980980
end
981+
982+
unless method_defined?(:kill) # RUBY_VERSION <= "3.3"
983+
def kill
984+
end
985+
end
981986
end
982987

983988
def setup

activerecord/test/cases/test_case.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,56 @@ class TestCase < ActiveSupport::TestCase # :nodoc:
3333
self.use_instantiated_fixtures = false
3434
self.use_transactional_tests = true
3535

36+
def after_teardown
37+
super
38+
check_connection_leaks
39+
end
40+
41+
def check_connection_leaks
42+
return if in_memory_db?
43+
44+
# Make sure tests didn't leave a connection owned by some background thread
45+
# which could lead to some slow wait in a subsequent thread.
46+
leaked_conn = []
47+
ActiveRecord::Base.connection_handler.each_connection_pool do |pool|
48+
# Ensure all in flights tasks are completed.
49+
# Otherwise they may still hold a connection.
50+
if pool.async_executor
51+
if pool.async_executor.scheduled_task_count != pool.async_executor.completed_task_count
52+
pool.connections.each do |conn|
53+
if conn.in_use? && conn.owner != Fiber.current && conn.owner != Thread.current
54+
if conn.owner.respond_to?(:join)
55+
conn.owner&.join(0.5)
56+
end
57+
end
58+
end
59+
end
60+
end
61+
62+
pool.reap
63+
pool.connections.each do |conn|
64+
if conn.in_use?
65+
if conn.owner != Fiber.current && conn.owner != Thread.current
66+
leaked_conn << [conn.owner, conn.owner.backtrace]
67+
conn.owner&.kill
68+
end
69+
conn.steal!
70+
pool.checkin(conn)
71+
end
72+
end
73+
end
74+
75+
if leaked_conn.size > 0
76+
puts "Found #{leaked_conn.size} leaked connections"
77+
leaked_conn.each do |owner, backtrace|
78+
puts "owner: #{owner}"
79+
puts "backtrace:\n#{backtrace}"
80+
puts
81+
end
82+
raise "Found #{leaked_conn.size} leaked connection after #{self.class.name}##{name}"
83+
end
84+
end
85+
3686
def create_fixtures(*fixture_set_names, &block)
3787
ActiveRecord::FixtureSet.create_fixtures(ActiveRecord::TestCase.fixture_paths, fixture_set_names, fixture_class_names, &block)
3888
end

0 commit comments

Comments
 (0)