Clean up leaked Playwright driver when browser launch fails in __aenter__ - #2160
Open
SohamKukreti wants to merge 1 commit into
Open
Clean up leaked Playwright driver when browser launch fails in __aenter__#2160SohamKukreti wants to merge 1 commit into
__aenter__#2160SohamKukreti wants to merge 1 commit into
Conversation
…__aenter__ Roll back partial startup in BrowserManager.start() so a failed browser launch stops the already-started driver process before re-raising. Fixes #2155
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2155
When
AsyncWebCrawler.__aenter__()fails partway through browser startup, the already-started Playwright driver process (node .../cli.js run-driver) was never cleaned up. Python does not call__aexit__for a failed__aenter__, so nothing stopped the driver — every failed startup attempt leaked one live child process. Retry loops made the process count grow linearly.Root cause
BrowserManager.start()initializes Playwright first and launches the browser later, with no error handling in between:Any failure after the first line (missing browser executable, broken WebKit deps, unreachable CDP endpoint, task cancellation) propagated out of
__aenter__and left the driver running.Fix
Wrap the startup body of
BrowserManager.start()intry/except BaseException:close()before re-raisingclose()intentionally returns early without stopping the driver (cdp_cleanup_on_close=Falsedefault), so the rollback additionally stops the driver explicitly — we own it even when we don't own the browserraise)BaseException(notException) so cancellation and Ctrl-C during startup also clean upReproduction
Verified with the issue's reproducer and with a natural failure (WebKit on Fedora, missing
libicudata.so.66), in a plain retry loop:List of files changed and why
crawl4ai/browser_manager.py- Wrap the body ofBrowserManager.start()intry/except BaseExceptionso a failed browser launch rolls back partial startup (stops the leaked Playwright driver process) before re-raising the original error. The diff is mostly indentation — review withgit diff -w; the real change is 15 added lines.tests/regression/test_reg_browser.py- Add regression testtest_failed_browser_launch_leaves_no_driverproving a failed launch propagates the original error, resets manager state, and leaves no driver child process.How Has This Been Tested?
start()branch: chromium/firefox/persistent-context/CDP launch failures, cancellation mid-start, cleanup that itself raises (original error not masked), retry-then-succeed on the same crawler instance, plus success paths (normal crawl, multi-arun,arun_many, explicitstart()/close(), sequential crawlers with driver-count baseline).pytest tests/regression/ -m "not network"— 293 passed, no new failures.playwright.stop()does not affect an external CDP browser (stays alive, answers CDP, accepts new clients) — it only ends the driver process we own.Checklist: