diff --git a/deploy/docker/api.py b/deploy/docker/api.py index 09a783939..d4865ec37 100644 --- a/deploy/docker/api.py +++ b/deploy/docker/api.py @@ -378,7 +378,7 @@ async def handle_markdown_request( cache_mode = CacheMode.ENABLED if cache == "1" else CacheMode.WRITE_ONLY - from crawler_pool import get_crawler, release_crawler + from crawler_pool import get_crawler, get_unpooled_crawler, release_crawler from utils import load_config as _load_config _cfg = _load_config() browser_cfg = BrowserConfig( @@ -604,13 +604,17 @@ def create_task_response(task: dict, task_id: str, base_url: str) -> dict: return response async def _dispose_crawler(crawler): - """Close a dedicated PDF crawler (not pooled) or release a pooled one.""" - from crawl4ai.processors.pdf import PDFCrawlerStrategy + """Close a crawler that was started outside the pool, or release a pooled one. + + Whoever starts a crawler outside the pool marks it `pooled = False`, so + this does not have to infer it from the strategy type - which only ever + recognised the PDF case. + """ from crawler_pool import release_crawler - if isinstance(crawler.crawler_strategy, PDFCrawlerStrategy): - await crawler.close() - else: + if getattr(crawler, "pooled", True): await release_crawler(crawler) + else: + await crawler.close() async def stream_results(crawler: AsyncWebCrawler, results_gen: AsyncGenerator) -> AsyncGenerator[bytes, None]: """Stream results with heartbeats and completion markers.""" @@ -698,7 +702,7 @@ async def handle_crawl_request( ) if config["crawler"]["rate_limiter"]["enabled"] else None ) - from crawler_pool import get_crawler, release_crawler + from crawler_pool import get_crawler, get_unpooled_crawler, release_crawler from crawl4ai.processors.pdf import PDFContentScrapingStrategy, PDFCrawlerStrategy is_pdf_crawl = isinstance(crawler_config.scraping_strategy, PDFContentScrapingStrategy) if is_pdf_crawl: @@ -710,6 +714,13 @@ async def handle_crawl_request( # Use PDFCrawlerStrategy when scraping PDFs, as headless Chromium can't render PDFs inline crawler = AsyncWebCrawler(crawler_strategy=PDFCrawlerStrategy()) await crawler.start() + crawler.pooled = False + elif hooks_config: + # A hook changes the browser context it runs in, and that change + # outlives the request: cookies it adds stay in the context, and + # the context is shared with whatever request comes next. So a + # request that brings hooks gets a browser of its own. + crawler = await get_unpooled_crawler(browser_config) else: crawler = await get_crawler(browser_config) @@ -880,10 +891,7 @@ async def handle_crawl_request( ) finally: if crawler: - if is_pdf_crawl: - await crawler.close() # not pooled; release_crawler would be a no-op - else: - await release_crawler(crawler) + await _dispose_crawler(crawler) async def handle_stream_crawl_request( urls: List[str], @@ -926,7 +934,7 @@ async def handle_stream_crawl_request( ), ) - from crawler_pool import get_crawler + from crawler_pool import get_crawler, get_unpooled_crawler from crawl4ai.processors.pdf import PDFContentScrapingStrategy, PDFCrawlerStrategy if isinstance(crawler_config.scraping_strategy, PDFContentScrapingStrategy): if hooks_config: @@ -937,6 +945,13 @@ async def handle_stream_crawl_request( # Use PDFCrawlerStrategy when scraping PDFs, as headless Chromium can't render PDFs inline crawler = AsyncWebCrawler(crawler_strategy=PDFCrawlerStrategy()) await crawler.start() + crawler.pooled = False + elif hooks_config: + # A hook changes the browser context it runs in, and that change + # outlives the request: cookies it adds stay in the context, and + # the context is shared with whatever request comes next. So a + # request that brings hooks gets a browser of its own. + crawler = await get_unpooled_crawler(browser_config) else: crawler = await get_crawler(browser_config) diff --git a/deploy/docker/crawler_pool.py b/deploy/docker/crawler_pool.py index 516d9562a..85fda8625 100644 --- a/deploy/docker/crawler_pool.py +++ b/deploy/docker/crawler_pool.py @@ -118,6 +118,27 @@ async def get_crawler(cfg: BrowserConfig) -> AsyncWebCrawler: USAGE_COUNT[sig] = 1 return crawler +async def get_unpooled_crawler(cfg: BrowserConfig) -> AsyncWebCrawler: + """Start a crawler for one request only, outside the pool. + + Hooks are attached to the crawler that serves a request, and what they do + to its browser context - cookies, headers, routes - outlives the request. + A pooled crawler would carry that into whatever comes next, so a request + that brings hooks gets a browser of its own. Close it when done; it is in + no pool, so the janitor will never come for it. + """ + mem_pct = get_container_memory_percent() + if mem_pct >= MEM_LIMIT: + logger.error(f"💥 Memory pressure: {mem_pct:.1f}% >= {MEM_LIMIT}%") + raise MemoryError(f"Memory at {mem_pct:.1f}%, refusing new browser") + + logger.info(f"🔒 Creating unpooled browser for hooked request (mem={mem_pct:.1f}%)") + crawler = AsyncWebCrawler(config=cfg, thread_safe=False) + await crawler.start() + crawler.pooled = False + return crawler + + async def release_crawler(crawler: AsyncWebCrawler): """Decrement active request count for a pooled crawler. diff --git a/tests/docker/test_pool_release.py b/tests/docker/test_pool_release.py index 6c81b3e52..6e25b1219 100644 --- a/tests/docker/test_pool_release.py +++ b/tests/docker/test_pool_release.py @@ -8,6 +8,8 @@ import pytest from unittest.mock import MagicMock +from crawl4ai import BrowserConfig + # --------------------------------------------------------------------------- # Standalone release_crawler implementation for testing @@ -153,3 +155,64 @@ async def test_janitor_safety_check(self): # Janitor check: now safe to close should_close = getattr(crawler, "active_requests", 0) == 0 assert should_close is True + + +# --------------------------------------------------------------------------- +# Crawlers started outside the pool +# +# Unlike the tests above, these import the real crawler_pool rather than a +# copy of its logic, so they fail if the module changes underneath them. +# --------------------------------------------------------------------------- + + +class TestUnpooledCrawler: + """A request that brings hooks must not be served from the shared pool. + + Hooks change the browser context they run in, and the change outlives the + request: cookies an add_cookies hook installs stay in the context, and the + context is handed to whatever request comes next. Scoping the hook itself + is not enough - measured on 0.9.3 with a per-request hook scope, the second + request's hook set was empty and it still received the first request's + cookie. + """ + + @pytest.mark.asyncio + async def test_unpooled_crawler_is_marked_and_not_registered(self, monkeypatch): + import crawler_pool + + started = {} + + class _FakeCrawler: + def __init__(self, config=None, thread_safe=False): + self.config = config + + async def start(self): + started["called"] = True + + monkeypatch.setattr(crawler_pool, "AsyncWebCrawler", _FakeCrawler) + monkeypatch.setattr(crawler_pool, "get_container_memory_percent", lambda: 10.0) + + before_hot = dict(crawler_pool.HOT_POOL) + before_cold = dict(crawler_pool.COLD_POOL) + + crawler = await crawler_pool.get_unpooled_crawler(BrowserConfig()) + + assert started["called"] is True + # The marker _dispose_crawler() reads to decide close-vs-release. + assert crawler.pooled is False + # And it is in no pool, so the janitor will never close it for us. + assert crawler_pool.HOT_POOL == before_hot + assert crawler_pool.COLD_POOL == before_cold + assert crawler not in crawler_pool.HOT_POOL.values() + assert crawler not in crawler_pool.COLD_POOL.values() + + @pytest.mark.asyncio + async def test_unpooled_crawler_refuses_under_memory_pressure(self, monkeypatch): + """Same guard the pool applies before starting any other browser.""" + import crawler_pool + + monkeypatch.setattr( + crawler_pool, "get_container_memory_percent", lambda: crawler_pool.MEM_LIMIT + 1 + ) + with pytest.raises(MemoryError): + await crawler_pool.get_unpooled_crawler(BrowserConfig())