diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 57a9dcf6d..dd6e054aa 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -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() @@ -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() @@ -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, @@ -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 diff --git a/playwright/_impl/_transport.py b/playwright/_impl/_transport.py index 3cc029e18..daeb5e3cf 100644 --- a/playwright/_impl/_transport.py +++ b/playwright/_impl/_transport.py @@ -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() @@ -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 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..89d20791b 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -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 @@ -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( - """ - - - """ - ) - 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__). @@ -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 == ""