Fix ConnectionPool::Reaper reaping parent connection pools after fork.

After forking, the connection handler will discard any connection pools that belongs to the parent process. However, it will continue to hold a reference to the connection pools of the parent process which is used for retrieval of the connection pool in the child process. Therefore, the `weakref_alive?` check in the connection pool reaper is insufficient since the connection pools of the parent process is never GCed.
This commit is contained in:
Guo Xiang Tan 2019-08-21 15:34:47 +08:00
parent 755da80063
commit 97fe59990c
2 changed files with 54 additions and 16 deletions

@ -336,7 +336,10 @@ def spawn_thread(frequency)
while running
sleep t
@mutex.synchronize do
@pools[frequency].select!(&:weakref_alive?)
@pools[frequency].select! do |pool|
pool.weakref_alive? && !pool.discarded?
end
@pools[frequency].each do |p|
p.reap
p.flush
@ -532,7 +535,7 @@ def disconnect!
# See AbstractAdapter#discard!
def discard! # :nodoc:
synchronize do
return if @connections.nil? # already discarded
return if self.discarded?
@connections.each do |conn|
conn.discard!
end
@ -540,6 +543,10 @@ def discard! # :nodoc:
end
end
def discarded? # :nodoc:
@connections.nil?
end
# Clears the cache which maps classes and re-connects connections that
# require reloading.
#
@ -648,7 +655,7 @@ def remove(conn)
# or a thread dies unexpectedly.
def reap
stale_connections = synchronize do
return unless @connections
return if self.discarded?
@connections.select do |conn|
conn.in_use? && !conn.owner.alive?
end.each do |conn|
@ -673,7 +680,7 @@ def flush(minimum_idle = @idle_timeout)
return if minimum_idle.nil?
idle_connections = synchronize do
return unless @connections
return if self.discarded?
@connections.select do |conn|
!conn.in_use? && conn.seconds_idle >= minimum_idle
end.each do |conn|

@ -5,23 +5,13 @@
module ActiveRecord
module ConnectionAdapters
class ReaperTest < ActiveRecord::TestCase
attr_reader :pool
def setup
super
@pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
end
teardown do
@pool.connections.each(&:close)
end
class FakePool
attr_reader :reaped
attr_reader :flushed
def initialize
def initialize(discarded: false)
@reaped = false
@discarded = discarded
end
def reap
@ -31,6 +21,14 @@ def reap
def flush
@flushed = true
end
def discard!
@discarded = true
end
def discarded?
@discarded
end
end
# A reaper with nil time should never reap connections
@ -40,6 +38,8 @@ def test_nil_time
reaper = ConnectionPool::Reaper.new(fp, nil)
reaper.run
assert_not fp.reaped
ensure
fp.discard!
end
def test_some_time
@ -53,10 +53,17 @@ def test_some_time
end
assert fp.reaped
assert fp.flushed
ensure
fp.discard!
end
def test_pool_has_reaper
spec = ActiveRecord::Base.connection_pool.spec.dup
pool = ConnectionPool.new spec
assert pool.reaper
ensure
pool.discard!
end
def test_reaping_frequency_configuration
@ -64,6 +71,8 @@ def test_reaping_frequency_configuration
spec.config[:reaping_frequency] = "10.01"
pool = ConnectionPool.new spec
assert_equal 10.01, pool.reaper.frequency
ensure
pool.discard!
end
def test_connection_pool_starts_reaper
@ -80,6 +89,8 @@ def test_connection_pool_starts_reaper
wait_for_conn_idle(conn)
assert_not_predicate conn, :in_use?
ensure
pool.discard!
end
def test_reaper_works_after_pool_discard
@ -136,6 +147,26 @@ def test_connection_pool_starts_reaper_in_fork
Process.waitpid(pid)
assert $?.success?
ensure
pool.discard!
end
def test_reaper_does_not_reap_discarded_connection_pools
discarded_pool = FakePool.new(discarded: true)
pool = FakePool.new
frequency = 0.001
ConnectionPool::Reaper.new(discarded_pool, frequency).run
ConnectionPool::Reaper.new(pool, frequency).run
until pool.flushed
Thread.pass
end
assert_not discarded_pool.reaped
assert pool.reaped
ensure
pool.discard!
end
def new_conn_in_thread(pool)