diff --git a/CHANGES.rst b/CHANGES.rst index cc99415b4f1..31d20d51bae 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,36 @@ .. towncrier release notes start +3.12.5 (2025-05-30) +=================== + +Features +-------- + +- Added ``ssl_shutdown_timeout`` parameter to :py:class:`~aiohttp.ClientSession` and :py:class:`~aiohttp.TCPConnector` to control the grace period for SSL shutdown handshake on TLS connections. This helps prevent "connection reset" errors on the server side while avoiding excessive delays during connector cleanup. Note: This parameter only takes effect on Python 3.11+ -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`11091`, :issue:`11094`. + + + + +Miscellaneous internal changes +------------------------------ + +- Improved performance of isinstance checks by using collections.abc types instead of typing module equivalents -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`11085`, :issue:`11088`. + + + + +---- + + 3.12.4 (2025-05-28) =================== diff --git a/CHANGES/11085.misc.rst b/CHANGES/11085.misc.rst deleted file mode 100644 index 67b1915cfcb..00000000000 --- a/CHANGES/11085.misc.rst +++ /dev/null @@ -1 +0,0 @@ -Improved performance of isinstance checks by using collections.abc types instead of typing module equivalents -- by :user:`bdraco`. diff --git a/CHANGES/11088.misc.rst b/CHANGES/11088.misc.rst deleted file mode 120000 index c9ebf3c31e1..00000000000 --- a/CHANGES/11088.misc.rst +++ /dev/null @@ -1 +0,0 @@ -11085.misc.rst \ No newline at end of file diff --git a/CHANGES/11100.bugfix.rst b/CHANGES/11100.bugfix.rst new file mode 100644 index 00000000000..a7c54059a14 --- /dev/null +++ b/CHANGES/11100.bugfix.rst @@ -0,0 +1,3 @@ +Fixed spurious "Future exception was never retrieved" warnings for connection lost errors when the connector is not closed -- by :user:`bdraco`. + +When connections are lost, the exception is now marked as retrieved since it is always propagated through other means, preventing unnecessary warnings in logs. diff --git a/aiohttp/client.py b/aiohttp/client.py index 20e7ce6cebb..b9dde9df6d0 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -297,6 +297,7 @@ def __init__( max_field_size: int = 8190, fallback_charset_resolver: _CharsetResolver = lambda r, b: "utf-8", middlewares: Sequence[ClientMiddlewareType] = (), + ssl_shutdown_timeout: Optional[float] = 0.1, ) -> None: # We initialise _connector to None immediately, as it's referenced in __del__() # and could cause issues if an exception occurs during initialisation. @@ -323,7 +324,7 @@ def __init__( self._timeout = timeout if connector is None: - connector = TCPConnector() + connector = TCPConnector(ssl_shutdown_timeout=ssl_shutdown_timeout) # Initialize these three attrs before raising any exception, # they are used in __del__ self._connector = connector diff --git a/aiohttp/client_proto.py b/aiohttp/client_proto.py index 4d559af0a78..be1e7672efe 100644 --- a/aiohttp/client_proto.py +++ b/aiohttp/client_proto.py @@ -98,6 +98,12 @@ def connection_lost(self, exc: Optional[BaseException]) -> None: ), original_connection_error, ) + # Mark the exception as retrieved to prevent + # "Future exception was never retrieved" warnings + # The exception is always passed on through + # other means, so this is safe + with suppress(Exception): + self.closed.exception() if self._payload_parser is not None: with suppress(Exception): # FIXME: log this somehow? diff --git a/aiohttp/connector.py b/aiohttp/connector.py index c525ed92191..4f3f22369a5 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -836,6 +836,12 @@ class TCPConnector(BaseConnector): socket_factory - A SocketFactoryType function that, if supplied, will be used to create sockets given an AddrInfoType. + ssl_shutdown_timeout - Grace period for SSL shutdown handshake on TLS + connections. Default is 0.1 seconds. This usually + allows for a clean SSL shutdown by notifying the + remote peer of connection closure, while avoiding + excessive delays during connector cleanup. + Note: Only takes effect on Python 3.11+. """ allowed_protocol_schema_set = HIGH_LEVEL_SCHEMA_SET | frozenset({"tcp"}) @@ -858,6 +864,7 @@ def __init__( happy_eyeballs_delay: Optional[float] = 0.25, interleave: Optional[int] = None, socket_factory: Optional[SocketFactoryType] = None, + ssl_shutdown_timeout: Optional[float] = 0.1, ): super().__init__( keepalive_timeout=keepalive_timeout, @@ -889,6 +896,7 @@ def __init__( self._interleave = interleave self._resolve_host_tasks: Set["asyncio.Task[List[ResolveResult]]"] = set() self._socket_factory = socket_factory + self._ssl_shutdown_timeout = ssl_shutdown_timeout def _close_immediately(self) -> List[Awaitable[object]]: for fut in chain.from_iterable(self._throttle_dns_futures.values()): @@ -1131,6 +1139,13 @@ async def _wrap_create_connection( loop=self._loop, socket_factory=self._socket_factory, ) + # Add ssl_shutdown_timeout for Python 3.11+ when SSL is used + if ( + kwargs.get("ssl") + and self._ssl_shutdown_timeout is not None + and sys.version_info >= (3, 11) + ): + kwargs["ssl_shutdown_timeout"] = self._ssl_shutdown_timeout return await self._loop.create_connection(*args, **kwargs, sock=sock) except cert_errors as exc: raise ClientConnectorCertificateError(req.connection_key, exc) from exc @@ -1204,13 +1219,27 @@ async def _start_tls_connection( timeout.sock_connect, ceil_threshold=timeout.ceil_threshold ): try: - tls_transport = await self._loop.start_tls( - underlying_transport, - tls_proto, - sslcontext, - server_hostname=req.server_hostname or req.host, - ssl_handshake_timeout=timeout.total, - ) + # ssl_shutdown_timeout is only available in Python 3.11+ + if ( + sys.version_info >= (3, 11) + and self._ssl_shutdown_timeout is not None + ): + tls_transport = await self._loop.start_tls( + underlying_transport, + tls_proto, + sslcontext, + server_hostname=req.server_hostname or req.host, + ssl_handshake_timeout=timeout.total, + ssl_shutdown_timeout=self._ssl_shutdown_timeout, + ) + else: + tls_transport = await self._loop.start_tls( + underlying_transport, + tls_proto, + sslcontext, + server_hostname=req.server_hostname or req.host, + ssl_handshake_timeout=timeout.total, + ) except BaseException: # We need to close the underlying transport since # `start_tls()` probably failed before it had a diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 287eba0e89d..f9806fdf985 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -57,7 +57,8 @@ The client session supports the context manager protocol for self closing. read_bufsize=2**16, \ max_line_size=8190, \ max_field_size=8190, \ - fallback_charset_resolver=lambda r, b: "utf-8") + fallback_charset_resolver=lambda r, b: "utf-8", \ + ssl_shutdown_timeout=0.1) The class for creating client sessions and making requests. @@ -240,6 +241,16 @@ The client session supports the context manager protocol for self closing. .. versionadded:: 3.8.6 + :param float ssl_shutdown_timeout: Grace period for SSL shutdown handshake on TLS + connections (``0.1`` seconds by default). This usually provides sufficient time + to notify the remote peer of connection closure, helping prevent broken + connections on the server side, while minimizing delays during connector + cleanup. This timeout is passed to the underlying :class:`TCPConnector` + when one is created automatically. Note: This parameter only takes effect + on Python 3.11+. + + .. versionadded:: 3.12.5 + .. attribute:: closed ``True`` if the session has been closed, ``False`` otherwise. @@ -1169,7 +1180,7 @@ is controlled by *force_close* constructor's parameter). force_close=False, limit=100, limit_per_host=0, \ enable_cleanup_closed=False, timeout_ceil_threshold=5, \ happy_eyeballs_delay=0.25, interleave=None, loop=None, \ - socket_factory=None) + socket_factory=None, ssl_shutdown_timeout=0.1) Connector for working with *HTTP* and *HTTPS* via *TCP* sockets. @@ -1296,6 +1307,16 @@ is controlled by *force_close* constructor's parameter). .. versionadded:: 3.12 + :param float ssl_shutdown_timeout: Grace period for SSL shutdown on TLS + connections (``0.1`` seconds by default). This parameter balances two + important considerations: usually providing sufficient time to notify + the remote server (which helps prevent "connection reset" errors), + while avoiding unnecessary delays during connector cleanup. + The default value provides a reasonable compromise for most use cases. + Note: This parameter only takes effect on Python 3.11+. + + .. versionadded:: 3.12.5 + .. attribute:: family *TCP* socket family e.g. :data:`socket.AF_INET` or diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 9433ad2f2bb..ea288657912 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -12,6 +12,7 @@ import tarfile import time import zipfile +from contextlib import suppress from typing import ( Any, AsyncIterator, @@ -704,6 +705,70 @@ async def handler(request: web.Request) -> web.Response: assert txt == "Test message" +@pytest.mark.skipif( + sys.version_info < (3, 11), reason="ssl_shutdown_timeout requires Python 3.11+" +) +async def test_ssl_client_shutdown_timeout( + aiohttp_server: AiohttpServer, + ssl_ctx: ssl.SSLContext, + aiohttp_client: AiohttpClient, + client_ssl_ctx: ssl.SSLContext, +) -> None: + # Test that ssl_shutdown_timeout is properly used during connection closure + + connector = aiohttp.TCPConnector(ssl=client_ssl_ctx, ssl_shutdown_timeout=0.1) + + async def streaming_handler(request: web.Request) -> NoReturn: + # Create a streaming response that continuously sends data + response = web.StreamResponse() + await response.prepare(request) + + # Keep sending data until connection is closed + while True: + await response.write(b"data chunk\n") + await asyncio.sleep(0.01) # Small delay between chunks + + assert False, "not reached" + + app = web.Application() + app.router.add_route("GET", "/stream", streaming_handler) + server = await aiohttp_server(app, ssl=ssl_ctx) + client = await aiohttp_client(server, connector=connector) + + # Verify the connector has the correct timeout + assert connector._ssl_shutdown_timeout == 0.1 + + # Start a streaming request to establish SSL connection with active data transfer + resp = await client.get("/stream") + assert resp.status == 200 + + # Create a background task that continuously reads data + async def read_loop() -> None: + while True: + # Read "data chunk\n" + await resp.content.read(11) + + read_task = asyncio.create_task(read_loop()) + await asyncio.sleep(0) # Yield control to ensure read_task starts + + # Record the time before closing + start_time = time.monotonic() + + # Now close the connector while the stream is still active + # This will test the ssl_shutdown_timeout during an active connection + await connector.close() + + # Verify the connection was closed within a reasonable time + # Should be close to ssl_shutdown_timeout (0.1s) but allow some margin + elapsed = time.monotonic() - start_time + assert elapsed < 0.3, f"Connection closure took too long: {elapsed}s" + + read_task.cancel() + with suppress(asyncio.CancelledError): + await read_task + assert read_task.done(), "Read task should be cancelled after connection closure" + + async def test_ssl_client_alpn( aiohttp_server: AiohttpServer, aiohttp_client: AiohttpClient, diff --git a/tests/test_client_proto.py b/tests/test_client_proto.py index e5d62d1e467..fa39b38d45c 100644 --- a/tests/test_client_proto.py +++ b/tests/test_client_proto.py @@ -252,3 +252,22 @@ async def test_connection_lost_sets_transport_to_none( proto.connection_lost(OSError()) assert proto.transport is None + + +async def test_connection_lost_exception_is_marked_retrieved( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that connection_lost properly handles exceptions without warnings.""" + proto = ResponseHandler(loop=loop) + proto.connection_made(mock.Mock()) + + # Simulate an SSL shutdown timeout error + ssl_error = TimeoutError("SSL shutdown timed out") + proto.connection_lost(ssl_error) + + # Verify the exception was set on the closed future + assert proto.closed.done() + exc = proto.closed.exception() + assert exc is not None + assert "Connection lost: SSL shutdown timed out" in str(exc) + assert exc.__cause__ is ssl_error diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 4f22b6a3851..0f16a0c7735 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -347,6 +347,34 @@ async def test_create_connector( assert m.called +async def test_ssl_shutdown_timeout_passed_to_connector() -> None: + # Test default value + async with ClientSession() as session: + assert isinstance(session.connector, TCPConnector) + assert session.connector._ssl_shutdown_timeout == 0.1 + + # Test custom value + async with ClientSession(ssl_shutdown_timeout=1.0) as session: + assert isinstance(session.connector, TCPConnector) + assert session.connector._ssl_shutdown_timeout == 1.0 + + # Test None value + async with ClientSession(ssl_shutdown_timeout=None) as session: + assert isinstance(session.connector, TCPConnector) + assert session.connector._ssl_shutdown_timeout is None + + # Test that it doesn't affect when custom connector is provided + custom_conn = TCPConnector(ssl_shutdown_timeout=2.0) + async with ClientSession( + connector=custom_conn, ssl_shutdown_timeout=1.0 + ) as session: + assert session.connector is not None + assert isinstance(session.connector, TCPConnector) + assert ( + session.connector._ssl_shutdown_timeout == 2.0 + ) # Should use connector's value + + def test_connector_loop(loop: asyncio.AbstractEventLoop) -> None: with contextlib.ExitStack() as stack: another_loop = asyncio.new_event_loop() diff --git a/tests/test_connector.py b/tests/test_connector.py index 5a342ef9641..ab854545882 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -2061,6 +2061,104 @@ async def test_tcp_connector_ctor(loop: asyncio.AbstractEventLoop) -> None: await conn.close() +async def test_tcp_connector_ssl_shutdown_timeout( + loop: asyncio.AbstractEventLoop, +) -> None: + # Test default value + conn = aiohttp.TCPConnector() + assert conn._ssl_shutdown_timeout == 0.1 + await conn.close() + + # Test custom value + conn = aiohttp.TCPConnector(ssl_shutdown_timeout=1.0) + assert conn._ssl_shutdown_timeout == 1.0 + await conn.close() + + # Test None value + conn = aiohttp.TCPConnector(ssl_shutdown_timeout=None) + assert conn._ssl_shutdown_timeout is None + await conn.close() + + +@pytest.mark.skipif( + sys.version_info < (3, 11), reason="ssl_shutdown_timeout requires Python 3.11+" +) +async def test_tcp_connector_ssl_shutdown_timeout_passed_to_create_connection( + loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock +) -> None: + # Test that ssl_shutdown_timeout is passed to create_connection for SSL connections + conn = aiohttp.TCPConnector(ssl_shutdown_timeout=2.5) + + with mock.patch.object( + conn._loop, "create_connection", autospec=True, spec_set=True + ) as create_connection: + create_connection.return_value = mock.Mock(), mock.Mock() + + req = ClientRequest("GET", URL("https://example.com"), loop=loop) + + with closing(await conn.connect(req, [], ClientTimeout())): + assert create_connection.call_args.kwargs["ssl_shutdown_timeout"] == 2.5 + + await conn.close() + + # Test with None value + conn = aiohttp.TCPConnector(ssl_shutdown_timeout=None) + + with mock.patch.object( + conn._loop, "create_connection", autospec=True, spec_set=True + ) as create_connection: + create_connection.return_value = mock.Mock(), mock.Mock() + + req = ClientRequest("GET", URL("https://example.com"), loop=loop) + + with closing(await conn.connect(req, [], ClientTimeout())): + # When ssl_shutdown_timeout is None, it should not be in kwargs + assert "ssl_shutdown_timeout" not in create_connection.call_args.kwargs + + await conn.close() + + # Test that ssl_shutdown_timeout is NOT passed for non-SSL connections + conn = aiohttp.TCPConnector(ssl_shutdown_timeout=2.5) + + with mock.patch.object( + conn._loop, "create_connection", autospec=True, spec_set=True + ) as create_connection: + create_connection.return_value = mock.Mock(), mock.Mock() + + req = ClientRequest("GET", URL("http://example.com"), loop=loop) + + with closing(await conn.connect(req, [], ClientTimeout())): + # For non-SSL connections, ssl_shutdown_timeout should not be passed + assert "ssl_shutdown_timeout" not in create_connection.call_args.kwargs + + await conn.close() + + +@pytest.mark.skipif(sys.version_info >= (3, 11), reason="Test for Python < 3.11") +async def test_tcp_connector_ssl_shutdown_timeout_not_passed_pre_311( + loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock +) -> None: + # Test that ssl_shutdown_timeout is NOT passed to create_connection on Python < 3.11 + conn = aiohttp.TCPConnector(ssl_shutdown_timeout=2.5) + + with mock.patch.object( + conn._loop, "create_connection", autospec=True, spec_set=True + ) as create_connection: + create_connection.return_value = mock.Mock(), mock.Mock() + + # Test with HTTPS + req = ClientRequest("GET", URL("https://example.com"), loop=loop) + with closing(await conn.connect(req, [], ClientTimeout())): + assert "ssl_shutdown_timeout" not in create_connection.call_args.kwargs + + # Test with HTTP + req = ClientRequest("GET", URL("http://example.com"), loop=loop) + with closing(await conn.connect(req, [], ClientTimeout())): + assert "ssl_shutdown_timeout" not in create_connection.call_args.kwargs + + await conn.close() + + async def test_tcp_connector_allowed_protocols(loop: asyncio.AbstractEventLoop) -> None: conn = aiohttp.TCPConnector() assert conn.allowed_protocol_schema_set == {"", "tcp", "http", "https", "ws", "wss"} diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 906e9128995..6094cdcb894 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -2,6 +2,7 @@ import gc import socket import ssl +import sys import unittest from unittest import mock @@ -1044,13 +1045,23 @@ async def make_conn() -> aiohttp.TCPConnector: ) ) - tls_m.assert_called_with( - mock.ANY, - mock.ANY, - _SSL_CONTEXT_VERIFIED, - server_hostname="www.python.org", - ssl_handshake_timeout=mock.ANY, - ) + if sys.version_info >= (3, 11): + tls_m.assert_called_with( + mock.ANY, + mock.ANY, + _SSL_CONTEXT_VERIFIED, + server_hostname="www.python.org", + ssl_handshake_timeout=mock.ANY, + ssl_shutdown_timeout=0.1, + ) + else: + tls_m.assert_called_with( + mock.ANY, + mock.ANY, + _SSL_CONTEXT_VERIFIED, + server_hostname="www.python.org", + ssl_handshake_timeout=mock.ANY, + ) self.assertEqual(req.url.path, "/") self.assertEqual(proxy_req.method, "CONNECT")