[PR #10656/06db052e backport][3.12] Revert: Close the socket if there's a failure in start_connection() #10464 (#10658)

Author: patchback[bot]

CHANGES/10464.bugfix.rst

@@ -0,0 +1 @@
+10656.bugfix.rst

CHANGES/10617.bugfix.rst

@@ -0,0 +1 @@
+10656.bugfix.rst

CHANGES/10656.bugfix.rst

@@ -0,0 +1,3 @@
+Reverted explicitly closing sockets if an exception is raised during ``create_connection`` -- by :user:`bdraco`.
+
+This change originally appeared in aiohttp 3.11.13

aiohttp/connector.py

@@ -1126,7 +1126,6 @@ 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
@@ -1139,11 +1138,7 @@ async def _wrap_create_connection(
                     loop=self._loop,
                     socket_factory=self._socket_factory,
                 )
-                connection = await self._loop.create_connection(
-                    *args, **kwargs, sock=sock
-                )
-                sock = None
-                return connection
+                return await self._loop.create_connection(*args, **kwargs, sock=sock)
         except cert_errors as exc:
             raise ClientConnectorCertificateError(req.connection_key, exc) from exc
         except ssl_errors as exc:
@@ -1152,15 +1147,6 @@ 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.
-                try:
-                    sock.close()
-                except OSError as exc:
-                    raise client_error(req.connection_key, exc) from exc
 
     async def _wrap_existing_connection(
         self,

tests/test_connector.py

@@ -627,56 +627,6 @@ 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_closes_socket_on_error_results_in_another_error(
-    loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock
-) -> None:
-    """Test that when error occurs while closing the socket."""
-    req = ClientRequest("GET", URL("https://127.0.0.1:443"), loop=loop)
-    start_connection.return_value.close.side_effect = OSError(
-        1, "error from closing socket"
-    )
-
-    conn = aiohttp.TCPConnector()
-    with (
-        mock.patch.object(
-            conn._loop,
-            "create_connection",
-            autospec=True,
-            spec_set=True,
-            side_effect=ValueError,
-        ),
-        pytest.raises(aiohttp.ClientConnectionError, match="error from closing socket"),
-    ):
-        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: