From 76bf214d2878a8425802128ddc8105ddc51e7e86 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Tue, 5 Apr 2022 12:47:10 -0700 Subject: [PATCH] fix: clean up HTTP session and pool during tear down phase * Add unit tests for the change * Fix the unittest to test on the correct class * Make linter happy --- google/auth/transport/requests.py | 4 ++++ google/auth/transport/urllib3.py | 4 ++++ tests/transport/test_requests.py | 6 ++++++ tests/transport/test_urllib3.py | 12 ++++++++++++ 4 files changed, 26 insertions(+) diff --git a/google/auth/transport/requests.py b/google/auth/transport/requests.py index a55b5f57b..cf5f0b1a0 100644 --- a/google/auth/transport/requests.py +++ b/google/auth/transport/requests.py @@ -149,6 +149,10 @@ def __init__(self, session=None): self.session = session + def __del__(self): + if hasattr(self, "session") and self.session is not None: + self.session.close() + def __call__( self, url, diff --git a/google/auth/transport/urllib3.py b/google/auth/transport/urllib3.py index ad67327a4..4abc26b52 100644 --- a/google/auth/transport/urllib3.py +++ b/google/auth/transport/urllib3.py @@ -427,6 +427,10 @@ def __exit__(self, exc_type, exc_val, exc_tb): """Proxy to ``self.http``.""" return self.http.__exit__(exc_type, exc_val, exc_tb) + def __del__(self): + if hasattr(self, "http") and self.http is not None: + self.http.clear() + @property def headers(self): """Proxy to ``self.http``.""" diff --git a/tests/transport/test_requests.py b/tests/transport/test_requests.py index 60d44a5f4..9018e5c8d 100644 --- a/tests/transport/test_requests.py +++ b/tests/transport/test_requests.py @@ -51,6 +51,12 @@ def test_timeout(self): assert http.request.call_args[1]["timeout"] == 5 + def test_session_closed_on_del(self): + http = mock.create_autospec(requests.Session, instance=True) + request = google.auth.transport.requests.Request(http) + request.__del__() + http.close.assert_called_with() + class TestTimeoutGuard(object): def make_guard(self, *args, **kwargs): diff --git a/tests/transport/test_urllib3.py b/tests/transport/test_urllib3.py index 396961c39..eb82b7744 100644 --- a/tests/transport/test_urllib3.py +++ b/tests/transport/test_urllib3.py @@ -305,3 +305,15 @@ def test_configure_mtls_channel_without_client_cert_env( is_mtls = authed_http.configure_mtls_channel(callback) assert not is_mtls get_client_cert_and_key.assert_not_called() + + def test_clear_pool_on_del(self): + http = mock.create_autospec(urllib3.PoolManager) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + mock.sentinel.credentials, http=http + ) + authed_http.__del__() + http.clear.assert_called_with() + + authed_http.http = None + authed_http.__del__() + # Expect it to not crash