From 411ef48e5bca1ed66a2e4dd7ecd8695e2bf6c94e Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 22 Jun 2018 16:49:03 -0700 Subject: [PATCH] Stop leaking quite so many connections While investigating the failures when you move func tests to py3, I noticed a whole bunch of ResourceWarning: unclosed noise. This should fix it. While we're at it, make get_capabilities less stupid. Change-Id: I3913e9334090b04a78143e0b70f621aad30fc642 Related-Change: I86d24104033b490a35178fc504d88c1e4a566628 --- swiftclient/client.py | 28 ++++++++++++++++------------ tests/functional/test_swiftclient.py | 1 + tests/unit/test_swiftclient.py | 8 +++++++- tests/unit/utils.py | 11 +++++++++-- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/swiftclient/client.py b/swiftclient/client.py index d843aecc..049b4afc 100644 --- a/swiftclient/client.py +++ b/swiftclient/client.py @@ -402,6 +402,7 @@ class HTTPConnection(object): self.request_session = requests.Session() # Don't use requests's default headers self.request_session.headers = None + self.resp = None if self.parsed_url.scheme not in ('http', 'https'): raise ClientException('Unsupported scheme "%s" in url "%s"' % (self.parsed_url.scheme, url)) @@ -506,6 +507,11 @@ class HTTPConnection(object): return self.resp + def close(self): + if self.resp: + self.resp.close() + self.request_session.close() + def http_connection(*arg, **kwarg): """:returns: tuple of (parsed url, connection object)""" @@ -527,6 +533,8 @@ def get_auth_1_0(url, user, key, snet, **kwargs): conn.request(method, parsed.path, '', headers) resp = conn.getresponse() body = resp.read() + resp.close() + conn.close() http_log((url, method,), headers, resp, body) url = resp.getheader('x-storage-url') @@ -1651,11 +1659,8 @@ class Connection(object): if (self.http_conn and isinstance(self.http_conn, tuple) and len(self.http_conn) > 1): conn = self.http_conn[1] - if hasattr(conn, 'close') and callable(conn.close): - # XXX: Our HTTPConnection object has no close, should be - # trying to close the requests.Session here? - conn.close() - self.http_conn = None + conn.close() + self.http_conn = None def get_auth(self): self.url, self.token = get_auth(self.authurl, self.user, self.key, @@ -1715,10 +1720,10 @@ class Connection(object): try: if not self.url or not self.token: self.url, self.token = self.get_auth() - self.http_conn = None + self.close() if self.service_auth and not self.service_token: self.url, self.service_token = self.get_service_auth() - self.http_conn = None + self.close() self.auth_end_time = time() if not self.http_conn: self.http_conn = self.http_connection() @@ -1908,8 +1913,7 @@ class Connection(object): url = url or self.url if not url: url, _ = self.get_auth() - scheme = urlparse(url).scheme - netloc = urlparse(url).netloc - url = scheme + '://' + netloc + '/info' - http_conn = self.http_connection(url) - return get_capabilities(http_conn) + parsed = urlparse(urljoin(url, '/info')) + if not self.http_conn: + self.http_conn = self.http_connection(url) + return get_capabilities((parsed, self.http_conn[1])) diff --git a/tests/functional/test_swiftclient.py b/tests/functional/test_swiftclient.py index 0380d961..1d76a8d1 100644 --- a/tests/functional/test_swiftclient.py +++ b/tests/functional/test_swiftclient.py @@ -108,6 +108,7 @@ class TestFunctional(unittest.TestCase): self.conn.delete_container(container) except swiftclient.ClientException: pass + self.conn.close() def _check_account_headers(self, headers): headers_to_check = [ diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py index f1147748..62875a5d 100644 --- a/tests/unit/test_swiftclient.py +++ b/tests/unit/test_swiftclient.py @@ -2532,6 +2532,9 @@ class TestConnection(MockHttpTest): def read(self, *args, **kwargs): return '' + def close(self): + pass + def local_http_connection(url, proxy=None, cacert=None, insecure=False, cert=None, cert_key=None, ssl_compression=True, timeout=None): @@ -2901,6 +2904,9 @@ class TestCloseConnection(MockHttpTest): self.assertIsNone(conn.http_conn) conn.close() self.assertIsNone(conn.http_conn) + # Can re-close + conn.close() + self.assertIsNone(conn.http_conn) def test_close_ok(self): url = 'http://www.test.com' @@ -2911,7 +2917,7 @@ class TestCloseConnection(MockHttpTest): self.assertEqual(len(conn.http_conn), 2) http_conn_obj = conn.http_conn[1] self.assertIsInstance(http_conn_obj, c.HTTPConnection) - self.assertFalse(hasattr(http_conn_obj, 'close')) + self.assertTrue(hasattr(http_conn_obj, 'close')) conn.close() diff --git a/tests/unit/utils.py b/tests/unit/utils.py index aab3b59c..8081501f 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -78,6 +78,10 @@ class StubResponse(object): self.body = body self.headers = headers or {} + def __repr__(self): + return '%s(%r, %r, %r)' % (self.__class__.__name__, self.status, + self.body, self.headers) + def fake_http_connect(*code_iter, **kwargs): """ @@ -102,7 +106,6 @@ def fake_http_connect(*code_iter, **kwargs): self.etag = etag self.content = self.body = body self.timestamp = timestamp - self._is_closed = True self.headers = headers or {} self.request = None @@ -162,6 +165,9 @@ def fake_http_connect(*code_iter, **kwargs): def getheader(self, name, default=None): return dict(self.getheaders()).get(name.lower(), default) + def close(self): + pass + timestamps_iter = iter(kwargs.get('timestamps') or ['1'] * len(code_iter)) etag_iter = iter(kwargs.get('etags') or [None] * len(code_iter)) x = kwargs.get('missing_container', [False] * len(code_iter)) @@ -228,7 +234,8 @@ class MockHttpTest(unittest.TestCase): parsed, _conn = _orig_http_connection(url, proxy=proxy) class RequestsWrapper(object): - pass + def close(self): + pass conn = RequestsWrapper() def request(method, path, *args, **kwargs):