Client close no join (#206)

* Correctly handle client disconnection - state can be left in libssh2 on disconnected sessions. Resolves #200
* Updated Changelog

Author: pkittenis

Changelog.rst

@@ -1,6 +1,15 @@
 Change Log
 ============
 
+1.11.2
+++++++
+
+Fixes
+------
+
+* `ParallelSSHClient.disconnect` would cause new client sessions to fail if `client.join` was not called prior - #200
+
+
 1.11.0
 ++++++
 

pssh/clients/native/single.py

@@ -136,17 +136,20 @@ def __init__(self, host,
     def disconnect(self):
         """Disconnect session, close socket if needed."""
         logger.debug("Disconnecting client for host %s", self.host)
+        self._keepalive_greenlet = None
         if self.session is not None:
             try:
                 self._eagain(self.session.disconnect)
             except Exception:
                 pass
-        if self.sock is not None and not self.sock.closed:
-            self.sock.close()
-            logger.debug("Client socket closed for host %s", self.host)
+            self.session = None
+        self.sock = None
 
     def __del__(self):
-        self.disconnect()
+        try:
+            self.disconnect()
+        except Exception:
+            pass
 
     def __enter__(self):
         return self
@@ -328,7 +331,7 @@ def execute(self, cmd, use_pty=False, channel=None):
         channel = self.open_session() if channel is None else channel
         if use_pty:
             self._eagain(channel.pty)
-        logger.debug("Executing command '%s'" % cmd)
+        logger.debug("Executing command '%s'", cmd)
         self._eagain(channel.execute, cmd)
         return channel
 

tests/embedded_server/openssh.py

@@ -41,9 +41,9 @@ def __init__(self, listen_ip='127.0.0.1', port=2222):
         self.listen_ip = listen_ip
         self.port = port
         self.server_proc = None
-        self.sshd_config = SSHD_CONFIG + '_%s' % ''.join(
-            random.choice(string.ascii_lowercase + string.digits)
-            for _ in range(8))
+        self.random_server = ''.join(random.choice(string.ascii_lowercase + string.digits)
+                                     for _ in range(8))
+        self.sshd_config = SSHD_CONFIG + '_%s' % self.random_server
         self._fix_masks()
         self.make_config()
 
@@ -60,7 +60,9 @@ def make_config(self):
         template = Template(tmpl)
         with open(self.sshd_config, 'w') as fh:
             fh.write(template.render(parent_dir=os.path.abspath(DIR_NAME),
-                                     listen_ip=self.listen_ip))
+                                     listen_ip=self.listen_ip,
+                                     random_server=self.random_server,
+            ))
             fh.write(os.linesep)
 
     def start_server(self):

tests/embedded_server/sshd_config.tmpl

@@ -7,3 +7,5 @@ ListenAddress {{listen_ip}}
 AcceptEnv LANG LC_*
 Subsystem sftp internal-sftp
 AuthorizedKeysFile {{parent_dir}}/authorized_keys
+MaxSessions 100
+PidFile {{random_server}}.pid

tests/test_native_parallel_client.py

@@ -1511,6 +1511,5 @@ def test_client_disconnect(self):
                                     return_list=True)
         client.join(output, consume_output=True)
         single_client = list(client._host_clients.values())[0]
-        self.assertFalse(single_client.sock.closed)
         del client
-        self.assertTrue(single_client.sock.closed)
+        self.assertEqual(single_client.session, None)

tests/test_native_single_client.py

@@ -156,3 +156,18 @@ def test_connection_timeout(self):
         # Should fail within greenlet timeout, otherwise greenlet will
         # raise timeout which will fail the test
         self.assertRaises(ConnectionErrorException, cmd.get, timeout=2)
+
+    def test_multiple_clients_exec_terminates_channels(self):
+        # See #200 - Multiple clients should not interfere with
+        # each other. session.disconnect can leave state in libssh2
+        # and break subsequent sessions even on different socket and
+        # session
+        for _ in range(5):
+            client = SSHClient(self.host, port=self.port,
+                               pkey=self.user_key,
+                               num_retries=1,
+                               allow_agent=False)
+            channel = client.execute(self.cmd)
+            output = list(client.read_output(channel))
+            self.assertListEqual(output, [b'me'])
+            client.disconnect()

tests/test_native_tunnel.py

@@ -206,7 +206,7 @@ def test_tunnel_channel_failure(self):
             tunnel.cleanup()
             spawn(proxy_client.execute, 'echo me')
             proxy_client.disconnect()
-            self.assertTrue(proxy_client.sock.closed)
+            self.assertEqual(proxy_client.sock, None)
         finally:
             remote_server.stop()