Skip to content
Closed
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
57 changes: 9 additions & 48 deletions playwright/_impl/_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,13 @@ async def _inner_send(
callback = self._connection._send_message_to_server(
self._object, method, augmented_params, timeout
)
try:
done, _ = await asyncio.wait(
{
self._connection._transport.on_error_future,
callback.future,
},
return_when=asyncio.FIRST_COMPLETED,
)
except asyncio.CancelledError as exc:
await self._connection._abort(
self._object,
callback,
str(exc) or "Task was cancelled",
)
raise
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()
Expand Down Expand Up @@ -249,10 +241,7 @@ def remove_listener(self, event: str, f: Any) -> None:


class ProtocolCallback:
def __init__(
self, loop: asyncio.AbstractEventLoop, id: int, no_reply: bool = False
) -> None:
self.id = id
def __init__(self, loop: asyncio.AbstractEventLoop, no_reply: bool = False) -> None:
self.stack_trace: traceback.StackSummary
self.no_reply = no_reply
self.future = loop.create_future()
Expand Down Expand Up @@ -404,7 +393,7 @@ def _send_message_to_server(
)
self._last_id += 1
id = self._last_id
callback = ProtocolCallback(self._loop, id, no_reply=no_reply)
callback = ProtocolCallback(self._loop, no_reply=no_reply)
task = asyncio.current_task(self._loop)
callback.stack_trace = cast(
traceback.StackSummary,
Expand Down Expand Up @@ -449,34 +438,6 @@ def _send_message_to_server(

return callback

async def _abort(
self, object: ChannelOwner, callback: ProtocolCallback, reason: str
) -> None:
try:
self._transport.send(
{
"guid": object._guid,
"method": "__abort__",
"params": {"id": callback.id, "reason": reason},
}
)
except (Error, OSError):
pass
try:
done, _ = await asyncio.wait(
{
self._transport.on_error_future,
callback.future,
},
return_when=asyncio.FIRST_COMPLETED,
)
finally:
if not callback.future.done():
callback.future.cancel()
for future in done:
if not future.cancelled():
future.exception()

def dispatch(self, msg: ParsedMessagePayload) -> None:
if self._closed_error:
return
Expand Down
12 changes: 8 additions & 4 deletions playwright/_impl/_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,19 @@ class PipeTransport(Transport):
def __init__(self, loop: asyncio.AbstractEventLoop) -> None:
super().__init__(loop)
self._stopped = False
self._output: Optional[asyncio.StreamWriter] = None
self._stopped_future: asyncio.Future = loop.create_future()

def request_stop(self) -> None:
assert self._output
self._stopped = True
self._output.close()
# May be called before connect() has spawned the driver.
if self._output:
self._output.close()

async def wait_until_stopped(self) -> None:
await self._stopped_future

async def connect(self) -> None:
self._stopped_future: asyncio.Future = asyncio.Future()

try:
# For pyinstaller and Nuitka
env = get_driver_env()
Expand All @@ -129,10 +130,13 @@ async def connect(self) -> None:
startupinfo=startupinfo,
)
except Exception as exc:
self._stopped_future.set_result(None)
self.on_error_future.set_exception(exc)
raise exc

self._output = self._proc.stdin
if self._stopped:
self.request_stop()

async def run(self) -> None:
assert self._proc.stdout
Expand Down
17 changes: 11 additions & 6 deletions playwright/async_api/_context_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,18 @@ async def __aenter__(self) -> AsyncPlaywright:
loop.create_task(self._connection.run())
playwright_future = self._connection.playwright_future

done, _ = await asyncio.wait(
{self._connection._transport.on_error_future, playwright_future},
return_when=asyncio.FIRST_COMPLETED,
)
if not playwright_future.done():
try:
done, _ = await asyncio.wait(
{self._connection._transport.on_error_future, playwright_future},
return_when=asyncio.FIRST_COMPLETED,
)
if not playwright_future.done():
playwright_future.cancel()
playwright = AsyncPlaywright(next(iter(done)).result())
except BaseException:
playwright_future.cancel()
playwright = AsyncPlaywright(next(iter(done)).result())
await self.__aexit__()
raise
playwright.stop = self.__aexit__ # type: ignore
return playwright

Expand Down
67 changes: 51 additions & 16 deletions tests/async/test_asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@


async def test_should_cancel_underlying_protocol_calls(
browser_name: str,
launch_arguments: Dict,
browser_name: str, launch_arguments: Dict
) -> None:
handler_exception = None

Expand All @@ -41,23 +40,12 @@ def exception_handler(loop: asyncio.AbstractEventLoop, context: Dict) -> None:
async with async_playwright() as p:
browser = await p[browser_name].launch(**launch_arguments)
page = await browser.new_page()
await page.set_content(
"""
<button disabled onclick="window.clicked = true">click me</button>
<script>window.clicked = false</script>
"""
)
task = asyncio.create_task(page.locator("button").click(timeout=0))
await page.wait_for_timeout(100)
assert not task.done()

task = asyncio.create_task(page.wait_for_selector("will-never-find"))
# make sure that the wait_for_selector message was sent to the server (driver)
await asyncio.sleep(0.1)
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task

await page.locator("button").evaluate("button => button.disabled = false")
await page.wait_for_timeout(700)
assert not await page.evaluate("window.clicked")
await browser.close()

# The actual 'Future exception was never retrieved' is logged inside the Future destructor (__del__).
Expand Down Expand Up @@ -154,3 +142,50 @@ async def test_should_return_proper_api_name_on_error(page: Page) -> None:
except Exception as error:
# Each browser returns slightly different error messages, but they should all start with "Page.evaluate:", because that was the Playwright method where the error originated
assert str(error).startswith("Page.evaluate:")


def test_cancelled_playwright_start_does_not_hang(tmp_path: Path) -> None:
# Regression test for https://github.com/microsoft/playwright/issues/42296.
# Cancelling __aenter__ left the driver and the transport tasks running,
# and asyncio.run() hung at loop shutdown.
script = tmp_path / "cancel_start.py"
script.write_text(
textwrap.dedent(
"""
import asyncio

from playwright.async_api import async_playwright


async def run_playwright():
async with async_playwright():
pass


async def main(delay):
task = asyncio.create_task(run_playwright())
await asyncio.sleep(delay)
task.cancel()
try:
await task
except asyncio.CancelledError:
pass


for delay in (0.001, 0.05, 0.5):
asyncio.run(main(delay))
print("DONE", flush=True)
"""
)
)
result = subprocess.run(
[sys.executable, str(script)],
capture_output=True,
text=True,
timeout=60,
)
assert result.returncode == 0, result.stderr
assert "DONE" in result.stdout
# Nothing is orphaned: no unretrieved futures, and the driver exits cleanly
# instead of dying with EPIPE mid-write.
assert result.stderr == ""