Skip to content

fix(deep_crawling): BFS/DFS/BestFirst strategies share one default FilterChain instance#2075

Open
chuenchen309 wants to merge 1 commit into
unclecode:developfrom
chuenchen309:fix/deep-crawl-shared-filter-chain-default
Open

fix(deep_crawling): BFS/DFS/BestFirst strategies share one default FilterChain instance#2075
chuenchen309 wants to merge 1 commit into
unclecode:developfrom
chuenchen309:fix/deep-crawl-shared-filter-chain-default

Conversation

@chuenchen309

Copy link
Copy Markdown

Summary

BFSDeepCrawlStrategy.__init__ and BestFirstCrawlingStrategy.__init__ both default filter_chain to FilterChain() — a classic Python mutable-default-argument bug. The default object is instantiated once at function-definition time, not per call, so every strategy instance created without an explicit filter_chain= argument shares the exact same FilterChain object. DFSDeepCrawlStrategy inherits BFS's __init__ unchanged, so it's affected too.

FilterChain is genuinely stateful: FilterChain.apply() increments self.stats._counters on every call, and add_filter() mutates self.filters in place. Two unrelated strategy instances (e.g. two concurrent crawls, or sequential crawls reusing default construction) silently share and corrupt each other's filter stats, and adding a filter to one instance's default chain leaks into every other instance using the default.

Confirmed via s1.filter_chain is s2.filter_chain == True for two independently-constructed BFSDeepCrawlStrategy instances before this fix.

Fix

filter_chain: Optional[FilterChain] = None, with self.filter_chain = filter_chain if filter_chain is not None else FilterChain() inside __init__, giving each instance a fresh chain when none is explicitly provided.

List of files changed and why

  • crawl4ai/deep_crawling/bfs_strategy.py — fix the mutable default (covers BFS and, via inheritance, DFS).
  • crawl4ai/deep_crawling/bff_strategy.py — same fix for BestFirstCrawlingStrategy.
  • tests/deep_crawling/test_default_filter_chain_isolation.py (new) — regression coverage.

How Has This Been Tested?

  • Added tests covering BFS, DFS, and BestFirst independently (filter_chain is not between two instances), plus a test confirming add_filter() on one instance's default chain doesn't leak into another's.
  • Confirmed red→green: reverting only the two source files reproduces s1.filter_chain is s2.filter_chain == True and the leaked-filter assertion failing; reapplying passes.
  • Full tests/deep_crawling/ suite: 73 passed.
  • No lint/format tooling is configured in this repo (checked pyproject.toml, no Makefile/.pre-commit-config.yaml); confirmed changed files compile cleanly with python -m py_compile.
  • xref: fix: RateLimiter burst race, Retry-After headers, deep crawl dispatcher (#1095) #1835 also touches both files but only adds a dispatcher= passthrough parameter, not the filter_chain default — no overlap.

Checklist:

  • I have performed a self-review of my own code
  • I have added/updated unit tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

AI-Generated disclosure

Found via an AI-assisted code review pass (Claude Code) over crawl4ai/deep_crawling/. I personally confirmed the shared-instance bug with a standalone repro, verified the fix across all three affected strategy classes, and ran the relevant test suite before submitting.

…lterChain instance

BFSDeepCrawlStrategy.__init__ and BestFirstCrawlingStrategy.__init__
both default filter_chain to `FilterChain()` -- a classic Python
mutable-default-argument bug. The default object is instantiated once
at function-definition time, not per call, so every strategy instance
created without an explicit filter_chain= argument shares the exact
same FilterChain object. DFSDeepCrawlStrategy inherits BFS's __init__
unchanged, so it's affected too.

FilterChain is genuinely stateful: FilterChain.apply() increments
self.stats._counters on every call, and add_filter() mutates
self.filters in place. Two unrelated strategy instances (e.g. two
concurrent crawls, or sequential crawls reusing default construction)
silently share and corrupt each other's filter stats, and adding a
filter to one instance's default chain leaks into every other instance
using the default. Confirmed via `s1.filter_chain is s2.filter_chain`
== True for two independently-constructed BFSDeepCrawlStrategy
instances before this fix.

Fix: filter_chain: Optional[FilterChain] = None, with
`self.filter_chain = filter_chain if filter_chain is not None else
FilterChain()` inside __init__, giving each instance a fresh chain
when none is explicitly provided.

Added tests/deep_crawling/test_default_filter_chain_isolation.py
covering BFS, DFS, and BestFirst independently, plus a test confirming
add_filter() on one instance's default chain doesn't leak into
another's. Full tests/deep_crawling/ suite: 73 passed. py_compile clean
on changed files (no lint/format tooling configured in this repo).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant