Skip to content

fix(browser): release get_page refcounts on cancel and defer full drains (#2419) - #2432

Open
Hugo-DDT wants to merge 1 commit into
unclecode:developfrom
Hugo-DDT:fix/2419-get-page-refcount-cancel
Open

Hugo-DDT wants to merge 1 commit into
unclecode:developfrom
Hugo-DDT:fix/2419-get-page-refcount-cancel

Conversation

@Hugo-DDT

Copy link
Copy Markdown

Summary

Fixes #2419.

BrowserManager.get_page() incremented a context refcount and then awaited new_page(), stealth setup, and browser recycle. Only Exception from new_page() undid the acquire. asyncio.CancelledError (raised when the Docker server hits wall_clock_s) and failures after new_page() leaked the refcount. Leaked entries never drained from _pending_cleanup, filled the 3-slot cap, and left later get_page() calls waiting inside recycle until they were cancelled too — a permanent Docker deadlock.

Changes

  1. Undo unreturned acquires — if get_page() never hands the page back, release the refcount and close any page we created (covers cancel and other BaseExceptions).
  2. Cancel-safe new_page() — shield the Playwright call and close a page that finishes after the caller was cancelled.
  3. Non-blocking recycle — when the pending drain is full, clear entries whose refcount is already 0; if still full, defer the bump instead of waiting inside get_page().

Test plan

  • pytest tests/browser/test_get_page_refcount_cancel_2419.py (6 new tests: cancel during new_page, stealth failure, cancel during bump, defer when drain full, clear drained then bump, successful handoff keeps refcount)
  • Existing TestSignatureShrink + TestLRUEviction still pass

Copilot AI balanced review requested due to automatic review settings October 22, 2026 09:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Cancellation or errors after the context refcount bump in get_page() used
to leak the acquire, which permanently filled the recycle pending queue and
deadlocked the Docker server under wall_clock_s cancels (unclecode#2419).

- Undo unreturned acquires (refcount + page) on BaseException
- Shield new_page so a late Playwright page is closed after cancel
- Make recycle bump non-blocking: clear drained pending entries, else defer
@Hugo-DDT
Hugo-DDT force-pushed the fix/2419-get-page-refcount-cancel branch from 9556406 to 73d2837 Compare October 23, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants