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

**This is a backport of PR #10656 as merged into master
(06db052).**

Reverts #10464

While this change improved the situation for uvloop users, it caused a
regression with `SelectorEventLoop` (issue #10617)

The alternative fix is MagicStack/uvloop#646
(not merged at the time of this PR)

issue #10617 appears to be very similar to
python/cpython@d5aeccf

If someone can come up with a working reproducer for #10617 we can
revisit this.
cc @top-oai

Minimal implementation that shows on cancellation the socket is cleaned
up without the explicit `close`
#10617 (comment)
so this should be unneeded unless I've missed something (very possible
with all the moving parts here)

## Related issue number

fixes #10617

Co-authored-by: J. Nick Koston <[email protected]>

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

@@ -1108,7 +1108,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
@@ -1120,11 +1119,7 @@ async def _wrap_create_connection(
                     interleave=self._interleave,
                     loop=self._loop,
                 )
-                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:
@@ -1133,15 +1128,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

@@ -617,56 +617,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: