Use WeakRef to avoid leaking connection pools

Prior to 3e2e8eeb9ea552bd4782538cf9348455f3d0e14a the Reaper thread
would hold a reference to connection pools indefinitely, preventing the
connection pool from being garbage collected, and also leaking the
Thread.

Since 3e2e8eeb9ea552bd4782538cf9348455f3d0e14a, there is only one Reaper
Thread for all pools, however all pools are still stored in a class
variable, preventing them from being garbage collected.

This commit instead holds reference to the pools through a WeakRef. This
way, connection pools referenced elsewhere will be reaped, any others
will be able to be garbage collected.

I don't love resorting to WeakRef to solve this, but I believe it's the
simplest way to accomplish the the desired behaviour.

1 files changed, 21 insertions, 11 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 4ff3cb0071..7628ef5537 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -3,6 +3,7 @@
 require "thread"
 require "concurrent/map"
 require "monitor"
+require "weakref"
 
 module ActiveRecord
   # Raised when a connection could not be obtained within the connection
@@ -294,28 +295,37 @@ module ActiveRecord
           @frequency = frequency
         end
 
-        @@mutex = Mutex.new
-        @@pools = {}
+        @mutex = Mutex.new
+        @pools = {}
 
-        def self.register_pool(pool, frequency) # :nodoc:
-          @@mutex.synchronize do
-            if @@pools.key?(frequency)
-              @@pools[frequency] << pool
-            else
-              @@pools[frequency] = [pool]
+        class << self
+          def register_pool(pool, frequency) # :nodoc:
+            @mutex.synchronize do
+              unless @pools.key?(frequency)
+                @pools[frequency] = []
+                spawn_thread(frequency)
+              end
+              @pools[frequency] << WeakRef.new(pool)
+            end
+          end
+
+          private
+
+            def spawn_thread(frequency)
               Thread.new(frequency) do |t|
                 loop do
                   sleep t
-                  @@mutex.synchronize do
-                    @@pools[frequency].each do |p|
+                  @mutex.synchronize do
+                    @pools[frequency].select!(&:weakref_alive?)
+                    @pools[frequency].each do |p|
                       p.reap
                       p.flush
+                    rescue WeakRef::RefError
                     end
                   end
                 end
               end
             end
-          end
         end
 
         def run