From 68e4442ec720ca6730959681951f5862f97b5772 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 30 Nov 2012 11:50:11 +0000
Subject: Fix memory leak in development mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .../connection_adapters/abstract/connection_pool.rb   | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

(limited to 'activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb')

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 db0db272a6..b5a8011ca4 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -490,6 +490,9 @@ module ActiveRecord
     # 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 @@ module ActiveRecord
 
       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 @@ module ActiveRecord
       # 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 @@ module ActiveRecord
       # 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 @@ module ActiveRecord
       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
 
-- 
cgit v1.2.3