From 44b99d6c49423c8198fdbccc89bf8bfe25adec01 Mon Sep 17 00:00:00 2001 From: Ahmed TAHRI Date: Thu, 31 Oct 2024 07:56:20 +0100 Subject: [PATCH] :bug: fix async connection shutdown in HTTP/1.1 and HTTP/2 leaving a `asyncio.TransportSocket` and `_SelectorSocketTransport` partially closed --- CHANGES.rst | 5 ++++ src/urllib3/backend/_async/hface.py | 3 ++ src/urllib3/contrib/ssa/__init__.py | 46 +++++++++++++++++++++++++---- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 48cf0878d0..080c9291b7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,8 @@ +2.11.908 (2024-10-31) +===================== + +- Fixed async connection shutdown in HTTP/1.1 and HTTP/2 leaving a ``asyncio.TransportSocket`` and ``_SelectorSocketTransport`` partially closed. + 2.11.907 (2024-10-30) ===================== diff --git a/src/urllib3/backend/_async/hface.py b/src/urllib3/backend/_async/hface.py index ef4b03192d..1bd0fba9ed 100644 --- a/src/urllib3/backend/_async/hface.py +++ b/src/urllib3/backend/_async/hface.py @@ -1644,6 +1644,9 @@ async def close(self) -> None: # type: ignore[override] try: self.sock.close() + # this avoids having SelectorSocketTransport in "closing" state + # pending. Thus avoid a ResourceWarning. + await self.sock.wait_for_close() except OSError: pass diff --git a/src/urllib3/contrib/ssa/__init__.py b/src/urllib3/contrib/ssa/__init__.py index c44f091b2b..e05d497af9 100644 --- a/src/urllib3/contrib/ssa/__init__.py +++ b/src/urllib3/contrib/ssa/__init__.py @@ -73,6 +73,23 @@ def __init__( def fileno(self) -> int: return self._fileno if self._fileno is not None else self._sock.fileno() + async def wait_for_close(self) -> None: + if self._connect_called: + return + + if self._writer is None: + return + + is_ssl = self._writer.get_extra_info("ssl_object") is not None + + if is_ssl: + # Give the connection a chance to write any data in the buffer, + # and then forcibly tear down the SSL connection. + await asyncio.sleep(0) + self._writer.transport.abort() + + await self._writer.wait_closed() + def close(self) -> None: if self._writer is not None: self._writer.close() @@ -83,15 +100,21 @@ def close(self) -> None: # probably not just uvloop. uvloop_edge_case_bug = False + # keep track of our clean exit procedure + shutdown_called = False + close_called = False + if hasattr(self._sock, "shutdown"): try: self._sock.shutdown(SHUT_RD) + shutdown_called = True except TypeError: uvloop_edge_case_bug = True # uvloop don't support shutdown! and sometime does not support close()... # see https://github.com/jawah/niquests/issues/166 for ctx. try: self._sock.close() + close_called = True except TypeError: # last chance of releasing properly the underlying fd! try: @@ -101,6 +124,7 @@ def close(self) -> None: else: try: direct_sock.shutdown(SHUT_RD) + shutdown_called = True except OSError: warnings.warn( ( @@ -113,13 +137,25 @@ def close(self) -> None: ) finally: direct_sock.detach() - elif hasattr(self._sock, "close"): - self._sock.close() - # we have to force call close() on our sock object in UDP ctx. (even after shutdown) + + # we have to force call close() on our sock object (even after shutdown). # or we'll get a resource warning for sure! - if self.type == socket.SOCK_DGRAM and hasattr(self._sock, "close"): + if isinstance(self._sock, socket.socket) and hasattr(self._sock, "close"): if not uvloop_edge_case_bug: - self._sock.close() + try: + self._sock.close() + close_called = True + except OSError: + pass + + if not close_called or not shutdown_called: + # this branch detect whether we have an asyncio.TransportSocket instead of socket.socket. + if hasattr(self._sock, "_sock"): + try: + self._sock._sock.detach() + except (AttributeError, OSError): + pass + except OSError: pass