fix(asyncio): do not deadlock in atexit handler (#3056)

Author: dgozman

CLAUDE.md

@@ -59,3 +59,40 @@ It documents the full process: the upstream commit-range diff over `docs/src/api
 - New public methods on impl classes need a sync test mirror under `tests/sync/`.
 - Keep `expected_api_mismatch.txt` minimal — every entry needs a one-line rationale comment above it.
 - Prefer `locals_to_params(locals())` for forwarding optional kwargs to channel sends, matching the rest of the codebase.
+
+## Commit Convention
+
+Before committing, run `mypy playwright` and fix errors.
+
+Semantic commit messages: `label(scope): description`
+
+Labels: `fix`, `feat`, `chore`, `docs`, `test`, `devops`
+
+```bash
+git checkout -b fix-12345
+# ... make changes ...
+git add <changed-files>
+git commit -m "$(cat <<'EOF'
+fix(asyncio): do not deadlock in atexit handler
+
+Fixes: https://github.com/microsoft/playwright-python/issues/12345
+EOF
+)"
+git push origin fix-12345
+gh pr create --repo microsoft/playwright-python --head username:fix-12345 \
+  --title "fix(asyncio): do not deadlock in atexit handler" \
+  --body "$(cat <<'EOF'
+## Summary
+- <describe the change very! briefly>
+
+Fixes https://github.com/microsoft/playwright-python/issues/12345
+EOF
+)"
+```
+
+Never add Co-Authored-By agents in commit message.
+Never add "Generated with" in commit message.
+Never add test plan to PR description. Keep PR description short — a few bullet points at most.
+Branch naming for issue fixes: `fix-<issue-number>`
+
+**Never `git push` without an explicit instruction to push.** Applies even when a PR is already open for the branch — additional commits are immediately visible to reviewers. Commit locally, report what was committed, and wait. Only push when the user's message contains "push", "upload", "create PR", "ship it", or equivalent.

local-requirements.txt

@@ -1,3 +1,4 @@
+asyncio-atexit==1.0.1
 autobahn==23.1.2
 black==25.1.0
 build==1.3.0

playwright/_impl/_transport.py

@@ -137,38 +137,44 @@ async def connect(self) -> None:
     async def run(self) -> None:
         assert self._proc.stdout
         assert self._proc.stdin
-        while not self._stopped:
-            try:
-                buffer = await self._proc.stdout.readexactly(4)
-                if self._stopped:
-                    break
-                length = int.from_bytes(buffer, byteorder="little", signed=False)
-                buffer = bytes(0)
-                while length:
-                    to_read = min(length, 32768)
-                    data = await self._proc.stdout.readexactly(to_read)
+        try:
+            while not self._stopped:
+                try:
+                    buffer = await self._proc.stdout.readexactly(4)
+                    if self._stopped:
+                        break
+                    length = int.from_bytes(buffer, byteorder="little", signed=False)
+                    buffer = bytes(0)
+                    while length:
+                        to_read = min(length, 32768)
+                        data = await self._proc.stdout.readexactly(to_read)
+                        if self._stopped:
+                            break
+                        length -= to_read
+                        if len(buffer):
+                            buffer = buffer + data
+                        else:
+                            buffer = data
                     if self._stopped:
                         break
-                    length -= to_read
-                    if len(buffer):
-                        buffer = buffer + data
-                    else:
-                        buffer = data
-                if self._stopped:
-                    break
 
-                obj = self.deserialize_message(buffer)
-                self.on_message(obj)
-            except asyncio.IncompleteReadError:
-                if not self._stopped:
-                    self.on_error_future.set_exception(
-                        Exception("Connection closed while reading from the driver")
-                    )
-                break
-            await asyncio.sleep(0)
-
-        await self._proc.communicate()
-        self._stopped_future.set_result(None)
+                    obj = self.deserialize_message(buffer)
+                    self.on_message(obj)
+                except asyncio.IncompleteReadError:
+                    if not self._stopped:
+                        self.on_error_future.set_exception(
+                            Exception("Connection closed while reading from the driver")
+                        )
+                    break
+                await asyncio.sleep(0)
+
+            await self._proc.communicate()
+        finally:
+            # Release waiters on wait_until_stopped() even if this task was
+            # cancelled before reaching the end (e.g. by asyncio.run()'s
+            # task-cancellation phase that runs before asyncio-atexit hooks).
+            if not self._stopped_future.done():
+                self._stopped_future.set_result(None)
 
     def send(self, message: Dict) -> None:
         assert self._output

tests/async/test_asyncio.py

@@ -13,7 +13,10 @@
 # limitations under the License.
 import asyncio
 import gc
+import subprocess
 import sys
+import textwrap
+from pathlib import Path
 from typing import Dict
 
 import pytest
@@ -89,6 +92,46 @@ async def raise_exception() -> None:
     assert await page.evaluate("() => 11 * 11") == 121
 
 
+def test_stop_does_not_deadlock_with_asyncio_atexit(tmp_path: Path) -> None:
+    # Regression test for https://github.com/microsoft/playwright-python/issues/3004.
+    # asyncio.run() cancels all remaining tasks (including transport.run()) before
+    # calling loop.close(). asyncio-atexit hooks loop.close() to run async cleanup,
+    # so awaiting playwright.stop() at that point used to deadlock on a future that
+    # the (already cancelled) run task would never set.
+    script = tmp_path / "atexit_stop.py"
+    script.write_text(
+        textwrap.dedent(
+            """
+            import asyncio
+
+            import asyncio_atexit
+            from playwright.async_api import async_playwright
+
+
+            async def main():
+                pw = await async_playwright().start()
+                asyncio_atexit.register(lambda: stop(pw))
+
+
+            async def stop(pw):
+                await pw.stop()
+                print("STOPPED", flush=True)
+
+
+            asyncio.run(main())
+            """
+        )
+    )
+    result = subprocess.run(
+        [sys.executable, str(script)],
+        capture_output=True,
+        text=True,
+        timeout=30,
+    )
+    assert result.returncode == 0, result.stderr
+    assert "STOPPED" in result.stdout
+
+
 async def test_should_return_proper_api_name_on_error(page: Page) -> None:
     try:
         await page.evaluate("does_not_exist")