diff --git a/paramiko/client.py b/paramiko/client.py index ba7e77d3e4d5e7740b3a49fe0109c719ff51ac2f_cGFyYW1pa28vY2xpZW50LnB5..aa148f1460a084b2db2bb250142728d24130562d_cGFyYW1pa28vY2xpZW50LnB5 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -350,6 +350,10 @@ # 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 ba7e77d3e4d5e7740b3a49fe0109c719ff51ac2f_c2l0ZXMvd3d3L2NoYW5nZWxvZy5yc3Q=..aa148f1460a084b2db2bb250142728d24130562d_c2l0ZXMvd3d3L2NoYW5nZWxvZy5yc3Q= 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 ba7e77d3e4d5e7740b3a49fe0109c719ff51ac2f_dGVzdHMvdGVzdF9jbGllbnQucHk=..aa148f1460a084b2db2bb250142728d24130562d_dGVzdHMvdGVzdF9jbGllbnQucHk= 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -33,6 +33,7 @@ import weakref from tempfile import mkstemp +import pytest from pytest_relaxed import raises from mock import patch, Mock @@ -473,6 +474,23 @@ 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