From 6302e56d6c1fec048f6438d9f1ac6a8cfaed7eb9 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@hawthorn.email>
Date: Sun, 26 May 2019 13:26:19 -0700
Subject: [PATCH] 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.
---
 .../abstract/connection_pool.rb               | 32 ++++++++++++-------
 1 file 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 4ff3cb0071dc0..7628ef5537195 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 @@ def initialize(pool, frequency)
           @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