diff --git a/CHANGES/10464.bugfix.rst b/CHANGES/10464.bugfix.rst new file mode 120000 index 00000000000..18996eb3cac --- /dev/null +++ b/CHANGES/10464.bugfix.rst @@ -0,0 +1 @@ +10656.bugfix.rst \ No newline at end of file diff --git a/CHANGES/10617.bugfix.rst b/CHANGES/10617.bugfix.rst new file mode 120000 index 00000000000..18996eb3cac --- /dev/null +++ b/CHANGES/10617.bugfix.rst @@ -0,0 +1 @@ +10656.bugfix.rst \ No newline at end of file diff --git a/CHANGES/10656.bugfix.rst b/CHANGES/10656.bugfix.rst new file mode 100644 index 00000000000..ec3853107ad --- /dev/null +++ b/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 diff --git a/aiohttp/connector.py b/aiohttp/connector.py index e5cf3674cba..7420bd6070a 100644 --- a/aiohttp/connector.py +++ b/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, diff --git a/tests/test_connector.py b/tests/test_connector.py index a86a2417423..a3fffc447ae 100644 --- a/tests/test_connector.py +++ b/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: