Fix memory leak in development mode

Keying these hashes by klass causes reloadable classes to never get
freed. Thanks to @thedarkone for pointing this out in
the comments on 221571beb6b4bb7437989bdefaf421f993ab6002.

This doesn't seem to make a massive difference to performance.

Benchmark
---------

require 'active_record'
require 'benchmark/ips'

class Post < ActiveRecord::Base
  establish_connection adapter: 'sqlite3', database: ':memory:'
end

GC.disable

Benchmark.ips(20) do |r|
  r.report { Post.connection }
end

Before
------

Calculating -------------------------------------
                          5632 i/100ms
-------------------------------------------------
                       218671.0 (±1.9%) i/s -    4364800 in  19.969401s

After
-----

Calculating -------------------------------------
                          8743 i/100ms
-------------------------------------------------
                       206525.9 (±17.8%) i/s -    4039266 in  19.992590s

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

@@ -490,6 +490,9 @@ def checkout_and_verify(c)
     # determine the connection pool that they should use.
     class ConnectionHandler
       def initialize
+        # These hashes are keyed by klass.name, NOT klass. Keying them by klass
+        # alone would lead to memory leaks in development mode as all previous
+        # instances of the class would stay in memory.
         @owner_to_pool = Hash.new { |h,k| h[k] = {} }
         @class_to_pool = Hash.new { |h,k| h[k] = {} }
       end
@@ -508,7 +511,7 @@ def connection_pools
 
       def establish_connection(owner, spec)
         @class_to_pool.clear
-        owner_to_pool[owner] = ConnectionAdapters::ConnectionPool.new(spec)
+        owner_to_pool[owner.name] = ConnectionAdapters::ConnectionPool.new(spec)
       end
 
       # Returns true if there are any active connections among the connection
@@ -554,7 +557,7 @@ def connected?(klass)
       # can be used as an argument for establish_connection, for easily
       # re-establishing the connection.
       def remove_connection(owner)
-        if pool = owner_to_pool.delete(owner)
+        if pool = owner_to_pool.delete(owner.name)
           @class_to_pool.clear
           pool.automatic_reconnect = false
           pool.disconnect!
@@ -572,13 +575,13 @@ def remove_connection(owner)
       # but that's ok since the nil case is not the common one that we wish to optimise
       # for.
       def retrieve_connection_pool(klass)
-        class_to_pool[klass] ||= begin
+        class_to_pool[klass.name] ||= begin
           until pool = pool_for(klass)
             klass = klass.superclass
             break unless klass <= Base
           end
 
-          class_to_pool[klass] = pool
+          class_to_pool[klass.name] = pool
         end
       end
 
@@ -593,21 +596,21 @@ def class_to_pool
       end
 
       def pool_for(owner)
-        owner_to_pool.fetch(owner) {
+        owner_to_pool.fetch(owner.name) {
           if ancestor_pool = pool_from_any_process_for(owner)
             # A connection was established in an ancestor process that must have
             # subsequently forked. We can't reuse the connection, but we can copy
             # the specification and establish a new connection with it.
             establish_connection owner, ancestor_pool.spec
           else
-            owner_to_pool[owner] = nil
+            owner_to_pool[owner.name] = nil
           end
         }
       end
 
       def pool_from_any_process_for(owner)
-        owner_to_pool = @owner_to_pool.values.find { |v| v[owner] }
-        owner_to_pool && owner_to_pool[owner]
+        owner_to_pool = @owner_to_pool.values.find { |v| v[owner.name] }
+        owner_to_pool && owner_to_pool[owner.name]
       end
     end
 

activerecord/test/cases/connection_adapters/connection_handler_test.rb

@@ -4,8 +4,8 @@ module ActiveRecord
   module ConnectionAdapters
     class ConnectionHandlerTest < ActiveRecord::TestCase
       def setup
-        @klass    = Class.new(Base)
-        @subklass = Class.new(@klass)
+        @klass    = Class.new(Base)   { def self.name; 'klass';    end }
+        @subklass = Class.new(@klass) { def self.name; 'subklass'; end }
 
         @handler = ConnectionHandler.new
         @pool    = @handler.establish_connection(@klass, Base.connection_pool.spec)
@@ -36,13 +36,11 @@ def test_retrieve_connection_pool_uses_superclass_when_no_subclass_connection
       end
 
       def test_retrieve_connection_pool_uses_superclass_pool_after_subclass_establish_and_remove
-        @handler.establish_connection 'north america', Base.connection_pool.spec
-        assert_same @handler.retrieve_connection_pool(@klass),
-          @handler.retrieve_connection_pool(@subklass)
+        sub_pool = @handler.establish_connection(@subklass, Base.connection_pool.spec)
+        assert_same sub_pool, @handler.retrieve_connection_pool(@subklass)
 
         @handler.remove_connection @subklass
-        assert_same @handler.retrieve_connection_pool(@klass),
-          @handler.retrieve_connection_pool(@subklass)
+        assert_same @pool, @handler.retrieve_connection_pool(@subklass)
       end
 
       def test_connection_pools