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 b61a4da33a3..08e6ae275ed 100644 --- a/aiohttp/connector.py +++ b/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, diff --git a/tests/test_connector.py b/tests/test_connector.py index aac122c7119..c4019df3cdf 100644 --- a/tests/test_connector.py +++ b/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: