Merge pull request #487 from azrle/fix/pool_keepalive

Fix closed connection in pool

Author: leehambley

Vagrantfile

@@ -2,6 +2,11 @@ VAGRANTFILE_API_VERSION = "2"
 
 Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
   config.vm.box = 'hashicorp/precise64'
+  config.vm.provision "shell", inline: <<-SHELL
+  echo 'ClientAliveInterval 1' >> /etc/ssh/sshd_config
+  echo 'ClientAliveCountMax 1' >> /etc/ssh/sshd_config
+  service ssh restart
+  SHELL
 
   json_config_path = File.join("test", "boxes.json")
   list = File.open(json_config_path).read

lib/sshkit/backends/connection_pool/cache.rb

@@ -36,12 +36,12 @@ def push(conn)
   def evict
     # Peek at the first connection to see if it is still fresh. If so, we can
     # return right away without needing to use `synchronize`.
-    first_expires_at, _connection = connections.first
-    return if first_expires_at.nil? || fresh?(first_expires_at)
+    first_expires_at, first_conn = connections.first
+    return if (first_expires_at.nil? || fresh?(first_expires_at)) && !closed?(first_conn)
 
     connections.synchronize do
-      fresh, stale = connections.partition do |expires_at, _|
-        fresh?(expires_at)
+      fresh, stale = connections.partition do |expires_at, conn|
+        fresh?(expires_at) && !closed?(conn)
       end
       connections.replace(fresh)
       stale.each { |_, conn| closer.call(conn) }
@@ -71,6 +71,13 @@ def fresh?(expires_at)
   end
 
   def closed?(conn)
-    conn.respond_to?(:closed?) && conn.closed?
+    return true if conn.respond_to?(:closed?) && conn.closed?
+    # test if connection is alive
+    conn.process(0) if conn.respond_to?(:process)
+    return false
+  rescue IOError => e
+    # connection is closed by server
+    return true if e.message == 'closed stream'
+    raise
   end
 end

test/functional/backends/test_netssh.rb

@@ -212,6 +212,20 @@ def test_interaction_handler
         end.run
         assert_equal("Enter Data\nCaptured SOME DATA", captured_command_result)
       end
+
+      def test_connection_pool_keepalive
+        # ensure we enable connection pool
+        SSHKit::Backend::Netssh.pool.idle_timeout = 10
+        Netssh.new(a_host) do |_host|
+          test :false
+        end.run
+        sleep 2.5
+        captured_command_result = nil
+        Netssh.new(a_host) do |_host|
+          captured_command_result = capture(:echo, 'some_value')
+        end.run
+        assert_equal "some_value", captured_command_result
+      end
     end
 
   end