From 41032dffb7883f9a358344ea0bed8dc301a7a90c Mon Sep 17 00:00:00 2001 From: yen0304 Date: Thu, 18 Jun 2026 22:47:24 +0800 Subject: [PATCH 1/2] fix: avoid retaining a completed task per protocol message on Python 3.14 Each protocol send awaited the long-lived transport `on_error_future` together with its per-message callback future via `asyncio.wait(..., FIRST_COMPLETED)`. On Python 3.14 the new asyncio await-graph keeps strong references between a task and the futures it awaits, so every completed send task stayed registered in `on_error_future._asyncio_awaited_by` until that future resolved, i.e. until the whole Connection closed. Intercepting requests (context.route / page.route) spawns one such task per intercepted request, so memory grew linearly with the number of requests and was not released by closing the page, context, or browser. Forward transport errors to the pending callback future via a done callback instead of awaiting `on_error_future` directly, so each send's await graph stays local to its own short-lived future. The transport-error behaviour is preserved: a send still raises the transport error if the transport dies mid-flight, and the callback is removed in a finally block. Adds a regression test that intercepts many requests and asserts completed asyncio tasks are not retained after the contexts are closed. --- playwright/_impl/_connection.py | 35 +++++++++++++++++------- tests/async/test_browsercontext_route.py | 34 +++++++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) 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/tests/async/test_browsercontext_route.py b/tests/async/test_browsercontext_route.py index d629be467..5d65bd988 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 @@ -514,3 +515,36 @@ 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 one completed asyncio Task per + # protocol message 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, so every send task lingered in the long-lived `on_error_future`. + def done_task_count() -> int: + gc.collect() + return sum( + 1 for o in gc.get_objects() if isinstance(o, asyncio.Task) and o.done() + ) + + 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 = done_task_count() + for _ in range(5): + await navigate_with_route() + leaked = done_task_count() - baseline + assert leaked < 50, f"intercepting requests leaked {leaked} done asyncio tasks" From a72b6032a6feb324d8c3b08c0079e47a9b59c7a1 Mon Sep 17 00:00:00 2001 From: yen0304 Date: Fri, 19 Jun 2026 17:08:36 +0800 Subject: [PATCH 2/2] fix: also drop route-handler task retention in _race_with_page_close Following review feedback, the route-handler task (BrowserContext._on_route) was still retained on Python 3.14 because _race_with_page_close awaited the long-lived page close future via asyncio.wait([fut, target_closed_future]). Bridge the two awaitables through a short-lived local future so the route-handler task is no longer recorded in the page close future await-graph. Tightens the regression test to count the per-request internal tasks (Channel.send and _on_route) separately instead of using a loose total threshold. --- playwright/_impl/_network.py | 22 ++++++++++--- tests/async/test_browsercontext_route.py | 39 +++++++++++++++--------- 2 files changed, 42 insertions(+), 19 deletions(-) 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 5d65bd988..766d890a7 100644 --- a/tests/async/test_browsercontext_route.py +++ b/tests/async/test_browsercontext_route.py @@ -317,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"" @@ -520,15 +517,22 @@ async def _handler3(route: Route) -> None: async def test_route_should_not_leak_done_tasks( browser: Browser, server: Server ) -> None: - # Regression test: intercepting requests used to retain one completed asyncio Task per - # protocol message until the entire connection closed. This is observable on Python 3.14, + # 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, so every send task lingered in the long-lived `on_error_future`. - def done_task_count() -> int: + # 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() - return sum( - 1 for o in gc.get_objects() if isinstance(o, asyncio.Task) and o.done() - ) + 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() @@ -543,8 +547,13 @@ async def handler(route: Route) -> None: ) await context.close() - baseline = done_task_count() + baseline = retained_request_tasks() for _ in range(5): await navigate_with_route() - leaked = done_task_count() - baseline - assert leaked < 50, f"intercepting requests leaked {leaked} done asyncio tasks" + 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"