Revert: Close the socket if there's a failure in start_connection() #10464

fixes #10617

alternative fix is MagicStack/uvloop#646

Author: bdraco

aiohttp/connector.py

@@ -1119,7 +1119,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
@@ -1132,11 +1131,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:
@@ -1145,15 +1140,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
 
     def _warn_about_tls_in_tls(
         self,

tests/test_connector.py

@@ -646,56 +646,6 @@ async def test_tcp_connector_certificate_error(
     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: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock
 ) -> None: