Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions playwright/_impl/_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 18 additions & 4 deletions playwright/_impl/_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
53 changes: 48 additions & 5 deletions tests/async/test_browsercontext_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

import asyncio
import gc
import re
from typing import Awaitable, Callable, List

Expand Down Expand Up @@ -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("""
<script>
(async () => {
await fetch('%s', {
Expand All @@ -326,9 +326,7 @@ async def test_should_override_post_body_with_empty_string(
});
})()
</script>
"""
% server.EMPTY_PAGE
),
""" % server.EMPTY_PAGE),
)

assert req[0].post_body == b""
Expand Down Expand Up @@ -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'<img src="{server.PREFIX}/missing-{i}.png">' 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"