Make enable_cleanup_closed a NOOP for Python 3.12.7+ and 3.13.1+ (#…

…9726)

CHANGES/9726.misc.rst

@@ -0,0 +1 @@
+Passing ``enable_cleanup_closed`` to :py:class:`aiohttp.TCPConnector` is now ignored on Python 3.12.7+ and 3.13.1+ since the underlying bug that caused asyncio to leak SSL connections has been fixed upstream -- by :user:`bdraco`.

aiohttp/connector.py

@@ -77,6 +77,14 @@
 HTTP_AND_EMPTY_SCHEMA_SET = HTTP_SCHEMA_SET | EMPTY_SCHEMA_SET
 HIGH_LEVEL_SCHEMA_SET = HTTP_AND_EMPTY_SCHEMA_SET | WS_SCHEMA_SET
 
+NEEDS_CLEANUP_CLOSED = (3, 13, 0) <= sys.version_info < (
+    3,
+    13,
+    1,
+) or sys.version_info < (3, 12, 7)
+# Cleanup closed is no longer needed after https://github.com/python/cpython/pull/118960
+# which first appeared in Python 3.12.7 and 3.13.1
+
 
 __all__ = ("BaseConnector", "TCPConnector", "UnixConnector", "NamedPipeConnector")
 
@@ -270,6 +278,17 @@ def __init__(
 
         # start cleanup closed transports task
         self._cleanup_closed_handle: Optional[asyncio.TimerHandle] = None
+
+        if enable_cleanup_closed and not NEEDS_CLEANUP_CLOSED:
+            warnings.warn(
+                "enable_cleanup_closed ignored because "
+                "https://github.com/python/cpython/pull/118960 is fixed in "
+                f"in Python version {sys.version_info}",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+            enable_cleanup_closed = False
+
         self._cleanup_closed_disabled = not enable_cleanup_closed
         self._cleanup_closed_transports: List[Optional[asyncio.Transport]] = []
 

docs/client_reference.rst

@@ -963,10 +963,14 @@ is controlled by *force_close* constructor's parameter).
                             connection releasing (optional).
 
    :param bool enable_cleanup_closed: some SSL servers do not properly complete
-      SSL shutdown process, in that case asyncio leaks ssl connections.
+      SSL shutdown process, in that case asyncio leaks SSL connections.
       If this parameter is set to True, aiohttp additionally aborts underlining
       transport after 2 seconds. It is off by default.
 
+      For Python version 3.12.7+, or 3.13.1 and later,
+      this parameter is ignored because the asyncio SSL connection
+      leak is fixed in these versions of Python.
+
 
    :param loop: :ref:`event loop<asyncio-event-loop>`
       used for handling connections.

tests/conftest.py

@@ -7,7 +7,7 @@
 from hashlib import md5, sha1, sha256
 from pathlib import Path
 from tempfile import TemporaryDirectory
-from typing import Any, Callable, Iterator
+from typing import Any, Callable, Generator, Iterator
 from unittest import mock
 from uuid import uuid4
 
@@ -239,3 +239,14 @@ def key(key_data: bytes) -> bytes:
 @pytest.fixture
 def ws_key(key: bytes) -> str:
     return base64.b64encode(sha1(key + WS_KEY).digest()).decode()
+
+
+@pytest.fixture
+def enable_cleanup_closed() -> Generator[None, None, None]:
+    """Fixture to override the NEEDS_CLEANUP_CLOSED flag.
+
+    On Python 3.12.7+ and 3.13.1+ enable_cleanup_closed is not needed,
+    however we still want to test that it works.
+    """
+    with mock.patch("aiohttp.connector.NEEDS_CLEANUP_CLOSED", True):
+        yield

tests/test_connector.py

@@ -377,6 +377,7 @@ async def test_get_expired(loop: asyncio.AbstractEventLoop) -> None:
     await conn.close()
 
 
+@pytest.mark.usefixtures("enable_cleanup_closed")
 async def test_get_expired_ssl(loop: asyncio.AbstractEventLoop) -> None:
     conn = aiohttp.BaseConnector(enable_cleanup_closed=True)
     key = ConnectionKey("localhost", 80, True, False, None, None, None)
@@ -440,6 +441,7 @@ async def test_release(loop: asyncio.AbstractEventLoop, key: ConnectionKey) -> N
         await conn.close()
 
 
+@pytest.mark.usefixtures("enable_cleanup_closed")
 async def test_release_ssl_transport(
     loop: asyncio.AbstractEventLoop, ssl_key: ConnectionKey
 ) -> None:
@@ -1797,6 +1799,7 @@ async def test_close_during_connect(loop: asyncio.AbstractEventLoop) -> None:
         assert proto.close.called
 
 
+@pytest.mark.usefixtures("enable_cleanup_closed")
 async def test_ctor_cleanup() -> None:
     loop = mock.Mock()
     loop.time.return_value = 1.5
@@ -1828,6 +1831,7 @@ async def test_cleanup(key: ConnectionKey) -> None:
     assert conn._cleanup_handle is None
 
 
+@pytest.mark.usefixtures("enable_cleanup_closed")
 async def test_cleanup_close_ssl_transport(
     loop: asyncio.AbstractEventLoop, ssl_key: ConnectionKey
 ) -> None:
@@ -1897,6 +1901,7 @@ async def test_cleanup3(loop: asyncio.AbstractEventLoop, key: ConnectionKey) ->
     await conn.close()
 
 
+@pytest.mark.usefixtures("enable_cleanup_closed")
 async def test_cleanup_closed(
     loop: asyncio.AbstractEventLoop, mocker: MockerFixture
 ) -> None:
@@ -1916,6 +1921,15 @@ async def test_cleanup_closed(
     assert cleanup_closed_handle.cancel.called
 
 
+async def test_cleanup_closed_is_noop_on_fixed_cpython() -> None:
+    """Ensure that enable_cleanup_closed is a noop on fixed Python versions."""
+    with mock.patch("aiohttp.connector.NEEDS_CLEANUP_CLOSED", False), pytest.warns(
+        DeprecationWarning, match="cleanup_closed ignored"
+    ):
+        conn = aiohttp.BaseConnector(enable_cleanup_closed=True)
+        assert conn._cleanup_closed_disabled is True
+
+
 async def test_cleanup_closed_disabled(
     loop: asyncio.AbstractEventLoop, mocker: MockerFixture
 ) -> None:
@@ -2440,6 +2454,7 @@ async def test_close_abort_closed_transports(loop: asyncio.AbstractEventLoop) ->
     assert conn.closed
 
 
+@pytest.mark.usefixtures("enable_cleanup_closed")
 async def test_close_cancels_cleanup_closed_handle(
     loop: asyncio.AbstractEventLoop,
 ) -> None:

tests/test_proxy.py

@@ -5,6 +5,7 @@
 import unittest
 from unittest import mock
 
+import pytest
 from yarl import URL
 
 import aiohttp
@@ -421,6 +422,7 @@ async def make_conn() -> aiohttp.TCPConnector:
         autospec=True,
         spec_set=True,
     )
+    @pytest.mark.usefixtures("enable_cleanup_closed")
     def test_https_connect_fingerprint_mismatch(
         self, start_connection: mock.Mock, ClientRequestMock: mock.Mock
     ) -> None: