diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index dc58bdaad..b8d36e442 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -120,16 +120,31 @@ async def _inner_send( callback = self._connection._send_message_to_server( self._object, method, _augment_params(params, timeout_calculator) ) - done, _ = await asyncio.wait( - { - self._connection._transport.on_error_future, - callback.future, - }, - return_when=asyncio.FIRST_COMPLETED, - ) - if not callback.future.done(): - callback.future.cancel() - result = next(iter(done)).result() + # Wake up the pending send if the transport errors, but do not `await` the long-lived + # `on_error_future` directly. On Python 3.14 the asyncio await-graph keeps every waiting + # task in `on_error_future._asyncio_awaited_by` until that future resolves (i.e. the whole + # connection closes), leaking one task per protocol message. Forwarding the error via a + # done callback instead keeps each send's await graph local to its own short-lived future. + on_error_future = self._connection._transport.on_error_future + + def _interrupt_on_transport_error(_: "asyncio.Future") -> None: + if not callback.future.done(): + callback.future.cancel() + + if on_error_future.done(): + on_error_future.result() # raises the transport error, if any + on_error_future.add_done_callback(_interrupt_on_transport_error) + try: + result = await callback.future + except asyncio.CancelledError: + # The send was interrupted. If the transport errored, surface that error to match the + # previous `asyncio.wait({on_error_future, callback.future})` behaviour; otherwise let + # the caller's cancellation propagate. + if on_error_future.done() and not on_error_future.cancelled(): + raise cast(BaseException, on_error_future.exception()) + raise + finally: + on_error_future.remove_done_callback(_interrupt_on_transport_error) # Protocol now has named return values, assume result is one level deeper unless # there is explicit ambiguity. if not result: diff --git a/playwright/_impl/_network.py b/playwright/_impl/_network.py index 852b5fac7..19c87abf6 100644 --- a/playwright/_impl/_network.py +++ b/playwright/_impl/_network.py @@ -555,10 +555,24 @@ async def _race_with_page_close(self, future: Coroutine) -> None: getattr(asyncio.current_task(self._loop), "__pw_stack__", inspect.stack(0)), ) target_closed_future = self.request._target_closed_future() - await asyncio.wait( - [fut, target_closed_future], - return_when=asyncio.FIRST_COMPLETED, - ) + # Wait until either the action finishes or the page closes, but do not `await` the + # long-lived `target_closed_future` directly: on Python 3.14 its await graph would keep + # this route-handler task referenced until the page is collected. Bridge both into a + # short-lived local future via done callbacks instead. + if not (fut.done() or target_closed_future.done()): + waiter = self._loop.create_future() + + def _wake(_: "asyncio.Future") -> None: + if not waiter.done(): + waiter.set_result(None) + + fut.add_done_callback(_wake) + target_closed_future.add_done_callback(_wake) + try: + await waiter + finally: + fut.remove_done_callback(_wake) + target_closed_future.remove_done_callback(_wake) if fut.done() and fut.exception(): raise cast(BaseException, fut.exception()) if target_closed_future.done(): diff --git a/tests/async/test_browsercontext_route.py b/tests/async/test_browsercontext_route.py index d629be467..766d890a7 100644 --- a/tests/async/test_browsercontext_route.py +++ b/tests/async/test_browsercontext_route.py @@ -13,6 +13,7 @@ # limitations under the License. import asyncio +import gc import re from typing import Awaitable, Callable, List @@ -316,8 +317,7 @@ async def test_should_override_post_body_with_empty_string( req = await asyncio.gather( server.wait_for_request("/empty.html"), - page.set_content( - """ + page.set_content(""" - """ - % server.EMPTY_PAGE - ), + """ % server.EMPTY_PAGE), ) assert req[0].post_body == b"" @@ -514,3 +512,48 @@ async def _handler3(route: Route) -> None: await page.goto(server.EMPTY_PAGE) assert intercepted == [3, 2, 1] + + +async def test_route_should_not_leak_done_tasks( + browser: Browser, server: Server +) -> None: + # Regression test: intercepting requests used to retain a completed asyncio Task per + # intercepted request until the entire connection closed. This is observable on Python 3.14, + # which keeps strong references between tasks and the futures they await via the asyncio + # await-graph. Two per-request tasks leaked into long-lived futures: the protocol send + # (`Channel.send`, awaiting the transport `on_error_future`) and the route-handler task + # (`BrowserContext._on_route`, awaiting the page close future in `_race_with_page_close`). + def retained_request_tasks() -> int: + gc.collect() + count = 0 + for obj in gc.get_objects(): + if not (isinstance(obj, asyncio.Task) and obj.done()): + continue + qualname = getattr(obj.get_coro(), "__qualname__", "") + if qualname.endswith("Channel.send") or qualname.endswith("_on_route"): + count += 1 + return count + + async def navigate_with_route() -> None: + context = await browser.new_context() + + async def handler(route: Route) -> None: + await route.abort() + + await context.route("**/*", handler) + page = await context.new_page() + await page.set_content( + "".join(f'' for i in range(20)) + ) + await context.close() + + baseline = retained_request_tasks() + for _ in range(5): + await navigate_with_route() + leaked = retained_request_tasks() - baseline + # Each iteration intercepts 20 requests; before the fix this retained dozens of completed + # send/route tasks. A couple may still be in-flight at measurement time, so allow a small + # constant rather than the per-request growth that the leak produced. + assert ( + leaked <= 2 + ), f"intercepting requests retained {leaked} completed asyncio tasks"