From 61f8c30652bad1acb4831c4721786dec742ca005 Mon Sep 17 00:00:00 2001
From: Dominic Metzger <dominic@rightscale.com>
Date: Mon, 9 Feb 2015 15:41:12 -0800
Subject: [PATCH] limiting bindings of default_proc
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Otherwise the hash’s default_proc will keep references to locals in
method connection_for as well as the current instantiation of Net::HTTP:Persistent
---
 lib/net/http/persistent.rb       | 14 ++++++++++--
 test/test_net_http_persistent.rb | 38 +++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/lib/net/http/persistent.rb b/lib/net/http/persistent.rb
index 98e1490..7ccebe7 100644
--- a/lib/net/http/persistent.rb
+++ b/lib/net/http/persistent.rb
@@ -276,6 +276,16 @@ def self.detect_idle_timeout uri, max = 10
     return sleep_time unless $!
   end
 
+  ##
+  # Returns a new nested hash
+  #
+  # Using a class method to limit the bindings referenced by the
+  # hash's default_proc
+
+  def self.new_nested_hash
+    Hash.new { |h,k| h[k] = {} }
+  end
+
   ##
   # This client's OpenSSL::X509::Certificate
 
@@ -586,8 +596,8 @@ def cleanup(generation, thread = Thread.current,
   # Creates a new connection for +uri+
 
   def connection_for uri
-    Thread.current[@generation_key]     ||= Hash.new { |h,k| h[k] = {} }
-    Thread.current[@ssl_generation_key] ||= Hash.new { |h,k| h[k] = {} }
+    Thread.current[@generation_key]     ||= Net::HTTP::Persistent.new_nested_hash
+    Thread.current[@ssl_generation_key] ||= Net::HTTP::Persistent.new_nested_hash
     Thread.current[@request_key]        ||= Hash.new 0
     Thread.current[@timeout_key]        ||= Hash.new EPOCH
 
diff --git a/test/test_net_http_persistent.rb b/test/test_net_http_persistent.rb
index 2cc5f89..7c30bb1 100644
--- a/test/test_net_http_persistent.rb
+++ b/test/test_net_http_persistent.rb
@@ -96,7 +96,8 @@ class BasicConnection
     attr_accessor :started, :finished, :address, :port,
                   :read_timeout, :open_timeout
     attr_reader :req, :debug_output
-    def initialize
+    def initialize(*args) # swallowing args so that it can be used
+                          # as Net::HTTP::Persistent's http_class
       @started, @finished = 0, 0
       @address, @port = 'example.com', 80
     end
@@ -1423,6 +1424,41 @@ def test_retry_change_requests_equals
     assert @http.can_retry?(post)
   end
 
+  def test_cleanup
+    http = Net::HTTP::Persistent.new 'name'
+    def http.http_class; BasicConnection end
+    uri = URI.parse 'http://example/'
+
+    # lets make sure that the thread's local storage will get
+    # initialized by 'connection_for'
+    Thread.current[http.generation_key] = nil
+    Thread.current[http.ssl_generation_key] = nil
+
+    c1_object_id = http.connection_for(uri).object_id
+
+    # Reconnect
+    http.reconnect
+    http.connection_for uri
+
+    # Cleanup
+    http.cleanup(http.generation)
+
+    # Verify that the old connection object is not lingering around
+    ObjectSpace.garbage_collect
+
+    c1_cleaned_up = true
+    ObjectSpace.each_object do |o|
+      if c1_object_id == o.object_id
+        c1_cleaned_up = false
+      end
+    end
+
+    assert c1_cleaned_up, "connection not cleaned up"
+    assert_empty conns
+    assert_empty ssl_conns
+  end
+
+
   def test_shutdown
     ssl_conns
     c = connection