Merge branch '2.8' into 2.9

3 files changed, 29 insertions, 0 deletions

diff --git a/paramiko/client.py b/paramiko/client.py
index 80c956cd..c3cbcb9d 100644
--- a/paramiko/client.py
+++ b/paramiko/client.py
@@ -350,6 +350,10 @@ class SSHClient(ClosingContextManager):
                     # Break out of the loop on success
                     break
                 except socket.error as e:
+                    # As mentioned in socket docs it is better
+                    # to close sockets explicitly
+                    if sock:
+                        sock.close()
                     # Raise anything that isn't a straight up connection error
                     # (such as a resolution error)
                     if e.errno not in (ECONNREFUSED, EHOSTUNREACH):
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index ada0d86c..b6bcf72f 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -2,6 +2,13 @@
 Changelog
 =========
 
+- :bug:`1822` (via, and relating to, far too many other issues to mention here)
+  Update `~paramiko.client.SSHClient` so it explicitly closes its wrapped
+  socket object upon encountering socket errors at connection time. This should
+  help somewhat with certain classes of memory leaks, resource warnings, and/or
+  errors (though we hasten to remind everyone that Client and Transport have
+  their own ``.close()`` methods for use in non-error situations!). Patch
+  courtesy of ``@YoavCohen``.
 - bug:`1637` (via :issue:`1599`) Raise `SSHException` explicitly when blank
   private key data is loaded, instead of the natural result of ``IndexError``.
   This should help more bits of Paramiko or Paramiko-adjacent codebases to
diff --git a/tests/test_client.py b/tests/test_client.py
index 21694e28..42487529 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -33,6 +33,7 @@ import warnings
 import weakref
 from tempfile import mkstemp
 
+import pytest
 from pytest_relaxed import raises
 from mock import patch, Mock
 
@@ -462,6 +463,23 @@ class SSHClientTest(ClientTest):
 
         assert p() is None
 
+    @patch("paramiko.client.socket.socket")
+    @patch("paramiko.client.socket.getaddrinfo")
+    def test_closes_socket_on_socket_errors(self, getaddrinfo, mocket):
+        getaddrinfo.return_value = (
+            ("irrelevant", None, None, None, "whatever"),
+        )
+
+        class SocksToBeYou(socket.error):
+            pass
+
+        my_socket = mocket.return_value
+        my_socket.connect.side_effect = SocksToBeYou
+        client = SSHClient()
+        with pytest.raises(SocksToBeYou):
+            client.connect(hostname="nope")
+        my_socket.close.assert_called_once_with()
+
     def test_client_can_be_used_as_context_manager(self):
         """
         verify that an SSHClient can be used a context manager