From 38f368e1c24a8211be979812293072a98ff8972e Mon Sep 17 00:00:00 2001
From: Andrew Top <142360808+top-oai@users.noreply.github.com>
Date: Thu, 20 Feb 2025 11:07:07 -0800
Subject: [PATCH] Explicitly close the socket if there is a failure in
 start_connection() (#10464)

(cherry picked from commit 182198fccae2d71b857a0f26b18e7317fd6cff4f)
---
 CHANGES/10464.bugfix.rst |  1 +
 CONTRIBUTORS.txt         |  1 +
 aiohttp/connector.py     | 13 ++++++++++++-
 tests/conftest.py        |  1 +
 tests/test_connector.py  | 23 +++++++++++++++++++++++
 tests/test_proxy.py      |  1 +
 6 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 CHANGES/10464.bugfix.rst

diff --git a/CHANGES/10464.bugfix.rst b/CHANGES/10464.bugfix.rst
new file mode 100644
index 00000000000..4e21000a317
--- /dev/null
+++ b/CHANGES/10464.bugfix.rst
@@ -0,0 +1 @@
+Changed connection creation to explicitly close sockets if an exception is raised in the event loop's ``create_connection`` method -- by :user:`top-oai`.
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index fb1b87ccc9d..1f0d1e7d2d7 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -42,6 +42,7 @@ Andrej Antonov
 Andrew Leech
 Andrew Lytvyn
 Andrew Svetlov
+Andrew Top
 Andrew Zhou
 Andrii Soldatenko
 Anes Abismail
diff --git a/aiohttp/connector.py b/aiohttp/connector.py
index 7e0986df657..14433ba37e1 100644
--- a/aiohttp/connector.py
+++ b/aiohttp/connector.py
@@ -1108,6 +1108,7 @@ async def _wrap_create_connection(
         client_error: Type[Exception] = ClientConnectorError,
         **kwargs: Any,
     ) -> Tuple[asyncio.Transport, ResponseHandler]:
+        sock: Union[socket.socket, None] = None
         try:
             async with ceil_timeout(
                 timeout.sock_connect, ceil_threshold=timeout.ceil_threshold
@@ -1119,7 +1120,11 @@ async def _wrap_create_connection(
                     interleave=self._interleave,
                     loop=self._loop,
                 )
-                return await self._loop.create_connection(*args, **kwargs, sock=sock)
+                connection = await self._loop.create_connection(
+                    *args, **kwargs, sock=sock
+                )
+                sock = None
+                return connection
         except cert_errors as exc:
             raise ClientConnectorCertificateError(req.connection_key, exc) from exc
         except ssl_errors as exc:
@@ -1128,6 +1133,12 @@ async def _wrap_create_connection(
             if exc.errno is None and isinstance(exc, asyncio.TimeoutError):
                 raise
             raise client_error(req.connection_key, exc) from exc
+        finally:
+            if sock is not None:
+                # Will be hit if an exception is thrown before the event loop takes the socket.
+                # In that case, proactively close the socket to guard against event loop leaks.
+                # For example, see https://github.com/MagicStack/uvloop/issues/653.
+                sock.close()
 
     async def _wrap_existing_connection(
         self,
diff --git a/tests/conftest.py b/tests/conftest.py
index 44ae384b633..95a98cd4fc0 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -221,6 +221,7 @@ def start_connection():
         "aiohttp.connector.aiohappyeyeballs.start_connection",
         autospec=True,
         spec_set=True,
+        return_value=mock.create_autospec(socket.socket, spec_set=True, instance=True),
     ) as start_connection_mock:
         yield start_connection_mock
 
diff --git a/tests/test_connector.py b/tests/test_connector.py
index 483759a4180..e79b36a673d 100644
--- a/tests/test_connector.py
+++ b/tests/test_connector.py
@@ -617,6 +617,29 @@ async def certificate_error(*args, **kwargs):
     await conn.close()
 
 
+async def test_tcp_connector_closes_socket_on_error(
+    loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock
+) -> None:
+    req = ClientRequest("GET", URL("https://127.0.0.1:443"), loop=loop)
+
+    conn = aiohttp.TCPConnector()
+    with (
+        mock.patch.object(
+            conn._loop,
+            "create_connection",
+            autospec=True,
+            spec_set=True,
+            side_effect=ValueError,
+        ),
+        pytest.raises(ValueError),
+    ):
+        await conn.connect(req, [], ClientTimeout())
+
+    assert start_connection.return_value.close.called
+
+    await conn.close()
+
+
 async def test_tcp_connector_server_hostname_default(
     loop: Any, start_connection: mock.AsyncMock
 ) -> None:
diff --git a/tests/test_proxy.py b/tests/test_proxy.py
index 1679b68909f..83457de891f 100644
--- a/tests/test_proxy.py
+++ b/tests/test_proxy.py
@@ -207,6 +207,7 @@ async def make_conn():
         "aiohttp.connector.aiohappyeyeballs.start_connection",
         autospec=True,
         spec_set=True,
+        return_value=mock.create_autospec(socket.socket, spec_set=True, instance=True),
     )
     def test_proxy_connection_error(self, start_connection: Any) -> None:
         async def make_conn():