diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 57a9dcf6d..4b91730f1 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -353,7 +353,14 @@ async def init() -> None: await self._transport.connect() self._init_task = self._loop.create_task(init()) - await self._transport.run() + try: + await self._transport.run() + except asyncio.CancelledError: + # Cancelled at loop shutdown - release tasks waiting on replies + # that will never arrive. + if not self._closed_error: + self.cleanup() + raise def stop_sync(self) -> None: self._transport.request_stop() @@ -452,18 +459,19 @@ def _send_message_to_server( async def _abort( self, object: ChannelOwner, callback: ProtocolCallback, reason: str ) -> None: + if not self._closed_error: + try: + self._transport.send( + { + "guid": object._guid, + "method": "__abort__", + "params": {"id": callback.id, "reason": reason}, + } + ) + except (Error, OSError): + pass try: - self._transport.send( - { - "guid": object._guid, - "method": "__abort__", - "params": {"id": callback.id, "reason": reason}, - } - ) - except (Error, OSError): - pass - try: - done, _ = await asyncio.wait( + await asyncio.wait( { self._transport.on_error_future, callback.future, @@ -471,11 +479,12 @@ async def _abort( return_when=asyncio.FIRST_COMPLETED, ) finally: + # Retrieve exceptions even if the wait itself was cancelled. if not callback.future.done(): callback.future.cancel() - for future in done: - if not future.cancelled(): - future.exception() + for future in (callback.future, self._transport.on_error_future): + if future.done() and not future.cancelled(): + future.exception() def dispatch(self, msg: ParsedMessagePayload) -> None: if self._closed_error: diff --git a/playwright/_impl/_json_pipe.py b/playwright/_impl/_json_pipe.py index 41973b8c7..5ace4001e 100644 --- a/playwright/_impl/_json_pipe.py +++ b/playwright/_impl/_json_pipe.py @@ -42,12 +42,7 @@ def dispose(self) -> None: self.on_error_future.cancel() self._stopped_future.cancel() - async def wait_until_stopped(self) -> None: - await self._stopped_future - async def connect(self) -> None: - self._stopped_future: asyncio.Future = asyncio.Future() - def handle_message(message: Dict) -> None: if self._stop_requested: return diff --git a/playwright/_impl/_transport.py b/playwright/_impl/_transport.py index 3cc029e18..136ccddbd 100644 --- a/playwright/_impl/_transport.py +++ b/playwright/_impl/_transport.py @@ -50,6 +50,7 @@ def __init__(self, loop: asyncio.AbstractEventLoop) -> None: self._loop = loop self.on_message: Callable[[ParsedMessagePayload], None] = lambda _: None self.on_error_future: asyncio.Future = loop.create_future() + self._stopped_future: asyncio.Future = loop.create_future() @abstractmethod def request_stop(self) -> None: @@ -58,9 +59,8 @@ def request_stop(self) -> None: def dispose(self) -> None: pass - @abstractmethod async def wait_until_stopped(self) -> None: - pass + await self._stopped_future @abstractmethod async def connect(self) -> None: @@ -92,18 +92,15 @@ class PipeTransport(Transport): def __init__(self, loop: asyncio.AbstractEventLoop) -> None: super().__init__(loop) self._stopped = False + self._output: Optional[asyncio.StreamWriter] = None def request_stop(self) -> None: - assert self._output self._stopped = True - self._output.close() - - async def wait_until_stopped(self) -> None: - await self._stopped_future + # May be called before connect() has spawned the driver. + if self._output: + self._output.close() async def connect(self) -> None: - self._stopped_future: asyncio.Future = asyncio.Future() - try: # For pyinstaller and Nuitka env = get_driver_env() @@ -129,10 +126,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 diff --git a/playwright/async_api/_context_manager.py b/playwright/async_api/_context_manager.py index 0c93f7043..e86f225ac 100644 --- a/playwright/async_api/_context_manager.py +++ b/playwright/async_api/_context_manager.py @@ -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 diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 2c90cb494..9133b5875 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -144,6 +144,51 @@ async def stop(pw): assert "STOPPED" in result.stdout +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 + assert "Future exception was never retrieved" not in result.stderr + + async def test_should_return_proper_api_name_on_error(page: Page) -> None: try: await page.evaluate("does_not_exist")