Fixing #961 - closing the http connection after every xmlrpc call

There were apparently some circumstances that
resulted in the connection not being closed; this just closes
it every time if it's still open after the rpc call is complete.

Signed-off-by: Luke Kanies <luke@madstop.com>

3 files changed, 29 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 9da1919d8..1eb84a02c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,8 @@
 0.24.8
+    Fixing #944 - changing error message from warning to info - connection recycled 
+
+    Fixed #961 - puppetd creating too many/not closing TCP connections
+
     Fixed #1959 - Added column protection for environment schema migration
 
     Fixing #1869 - autoloaded files should never leak exceptions
diff --git a/lib/puppet/network/xmlrpc/client.rb b/lib/puppet/network/xmlrpc/client.rb
index 91bb03e1f..678ab6c00 100644
--- a/lib/puppet/network/xmlrpc/client.rb
+++ b/lib/puppet/network/xmlrpc/client.rb
@@ -128,7 +128,7 @@ module Puppet::Network
         end
 
         handle_error(Errno::EPIPE, EOFError) do |client, detail, namespace, method|
-            Puppet.warning "Other end went away; restarting connection and retrying"
+            Puppet.info "Other end went away; restarting connection and retrying"
             client.recycle_connection
             return :retry
         end
@@ -147,6 +147,8 @@ module Puppet::Network
             rescue Exception => detail
                 retry if self.class.error_handler(detail).execute(self, detail, namespace, method) == :retry
             end
+        ensure
+            http.finish if http.started?
         end
 
         def http
diff --git a/spec/unit/network/xmlrpc/client.rb b/spec/unit/network/xmlrpc/client.rb
index 76ef5c799..36e59429c 100755
--- a/spec/unit/network/xmlrpc/client.rb
+++ b/spec/unit/network/xmlrpc/client.rb
@@ -20,6 +20,28 @@ describe Puppet::Network::XMLRPCClient do
             @client.report("eh").should == "foo"
         end
 
+        it "should always close the http connection if it is still open after the call" do
+            http = mock 'http'
+            @client.stubs(:http).returns http
+
+            http.expects(:started?).returns true
+            http.expects(:finish)
+
+            @client.report("eh").should == "foo"
+        end
+
+        it "should always close the http connection if it is still open after a call that raises an exception" do
+            http = mock 'http'
+            @client.stubs(:http).returns http
+
+            @client.expects(:call).raises RuntimeError
+
+            http.expects(:started?).returns true
+            http.expects(:finish)
+
+            lambda { @client.report("eh") }.should raise_error
+        end
+
         describe "when returning the http instance" do
             it "should use the http pool to create the instance" do
                 @client.instance_variable_set("@http", nil)