From db4ba972516b2cc37cfa602782b57b0166a38b92 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 6 Oct 2026 14:34:12 -0700 Subject: [PATCH] fix(sync): raise instead of spinning after the driver exits The dispatcher fiber finishes once the connection to the driver ends. Switching to a dead greenlet returns immediately, so every sync wait loop degenerated into a 100% CPU busy loop that never raised. SyncBase._sync, EventInfo.value and DisposableStub._sync now share one wait helper that raises TargetClosedError when the dispatcher is gone. Reported in https://github.com/microsoft/playwright-python/pull/3187 --- playwright/_impl/_disposable.py | 17 ++++++------ playwright/_impl/_greenlets.py | 26 +++++++++++++++++ playwright/_impl/_sync_base.py | 14 ++++++---- tests/sync/test_sync.py | 49 +++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 14 deletions(-) diff --git a/playwright/_impl/_disposable.py b/playwright/_impl/_disposable.py index c080626ab..948f2f76b 100644 --- a/playwright/_impl/_disposable.py +++ b/playwright/_impl/_disposable.py @@ -12,14 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -import asyncio import traceback -from typing import Awaitable, Callable, Dict +from typing import Any, Awaitable, Callable, Coroutine, Dict import greenlet from playwright._impl._connection import ChannelOwner, _capture_stack_trace from playwright._impl._errors import Error, is_target_closed_error +from playwright._impl._greenlets import connection_closed_error, wait_for_future class Disposable(ChannelOwner): @@ -70,13 +70,16 @@ def __enter__(self) -> "DisposableStub": def __exit__(self, *args: object) -> None: self._sync(self.dispose()) - def _sync(self, coro: object) -> object: + def _sync(self, coro: Coroutine[Any, Any, Any]) -> object: __tracebackhide__ = True if self._loop.is_closed(): - coro.close() # type: ignore + coro.close() raise Error("Event loop is closed! Is Playwright already stopped?") + if self._dispatcher_fiber.dead: + coro.close() + raise connection_closed_error() g_self = greenlet.getcurrent() - task = self._loop.create_task(coro) # type: ignore + task = self._loop.create_task(coro) setattr( task, "__pw_stack__", @@ -84,9 +87,7 @@ def _sync(self, coro: object) -> object: ) setattr(task, "__pw_stack_trace__", traceback.extract_stack(limit=10)) task.add_done_callback(lambda _: g_self.switch()) - while not task.done(): - self._dispatcher_fiber.switch() # type: ignore - asyncio._set_running_loop(self._loop) + wait_for_future(self._loop, self._dispatcher_fiber, task) return task.result() async def close(self) -> None: diff --git a/playwright/_impl/_greenlets.py b/playwright/_impl/_greenlets.py index a381e6e53..606eb01ba 100644 --- a/playwright/_impl/_greenlets.py +++ b/playwright/_impl/_greenlets.py @@ -11,11 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import asyncio import os from typing import Tuple import greenlet +from playwright._impl._errors import TargetClosedError + def _greenlet_trace_callback( event: str, args: Tuple[greenlet.greenlet, greenlet.greenlet] @@ -47,3 +50,26 @@ def __str__(self) -> str: class EventGreenlet(greenlet.greenlet): def __str__(self) -> str: return "" + + +def connection_closed_error() -> TargetClosedError: + return TargetClosedError("Playwright connection closed") + + +def wait_for_future( + loop: asyncio.AbstractEventLoop, + dispatcher_fiber: greenlet.greenlet, + future: "asyncio.Future", +) -> None: + __tracebackhide__ = True + while not future.done(): + # The dispatcher fiber exits once the connection to the driver ends, e.g. + # when the driver process dies. Nothing can settle the future after that, + # and switching to a dead greenlet returns right away, so we would spin. + if dispatcher_fiber.dead: + future.cancel() + raise connection_closed_error() + dispatcher_fiber.switch() + # The loop is only running for as long as the dispatcher fiber is alive. + if not dispatcher_fiber.dead: + asyncio._set_running_loop(loop) diff --git a/playwright/_impl/_sync_base.py b/playwright/_impl/_sync_base.py index e13680b8c..53f895f76 100644 --- a/playwright/_impl/_sync_base.py +++ b/playwright/_impl/_sync_base.py @@ -32,6 +32,7 @@ import greenlet from playwright._impl._connection import _capture_stack_trace +from playwright._impl._greenlets import connection_closed_error, wait_for_future from playwright._impl._helper import Error from playwright._impl._impl_to_api_mapping import ImplToApiMapping, ImplWrapper @@ -51,9 +52,9 @@ def __init__(self, sync_base: "SyncBase", future: "asyncio.Future[T]") -> None: @property def value(self) -> T: - while not self._future.done(): - self._sync_base._dispatcher_fiber.switch() - asyncio._set_running_loop(self._sync_base._loop) + wait_for_future( + self._sync_base._loop, self._sync_base._dispatcher_fiber, self._future + ) exception = self._future.exception() if exception: raise exception @@ -102,6 +103,9 @@ def _sync( if self._loop.is_closed(): coro.close() raise Error("Event loop is closed! Is Playwright already stopped?") + if self._dispatcher_fiber.dead: + coro.close() + raise connection_closed_error() g_self = greenlet.getcurrent() task: asyncio.tasks.Task[Any] = self._loop.create_task(coro) @@ -109,9 +113,7 @@ def _sync( setattr(task, "__pw_stack_trace__", traceback.extract_stack(limit=10)) task.add_done_callback(lambda _: g_self.switch()) - while not task.done(): - self._dispatcher_fiber.switch() - asyncio._set_running_loop(self._loop) + wait_for_future(self._loop, self._dispatcher_fiber, task) return task.result() def _wrap_handler( diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index 7e3d36977..d80a5fa12 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -399,6 +399,55 @@ def test_should_not_orphan_callback_on_non_serializable_params( assert "Future exception was never retrieved" not in result.stderr +def test_sync_calls_should_raise_after_driver_exit( + browser_name: str, + launch_arguments: Dict[str, Any], + tmp_path: Path, +) -> None: + # Regression test for https://github.com/microsoft/playwright-python/pull/3187. + # Run in a subprocess with a timeout, the calls used to spin forever instead of raising. + script = tmp_path / "driver_exit.py" + script.write_text( + textwrap.dedent( + f""" + from playwright.sync_api import sync_playwright + + + def error_of(callback): + try: + callback() + except Exception as e: + return str(e) + return "" + + + with sync_playwright() as p: + browser = p[{browser_name!r}].launch(**{launch_arguments!r}) + page = browser.new_page() + route = page.route("**/*", lambda route: route.continue_()) + + def expect_event(): + with page.expect_event("console", timeout=0): + p._impl_obj._connection._transport._proc.kill() + + def dispose_route(): + with route: + pass + + for callback in [expect_event, page.title, dispose_route, browser.close]: + assert "Playwright connection closed" in error_of(callback), callback + """ + ) + ) + result = subprocess.run( + [sys.executable, str(script)], + capture_output=True, + text=True, + timeout=60, + ) + assert result.returncode == 0, result.stderr + + def test_click_should_accept_timedelta_for_timeout(page: Page) -> None: with pytest.raises(TimeoutError, match="Timeout 1ms exceeded"): page.click("does-not-exist", timeout=timedelta(milliseconds=1))