fix infinite recursion in BaseModel equality checks #11581
Conversation
CodSpeed Performance ReportMerging #11581 will not alter performanceComparing Summary
|
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
|
Thanks for the contribution, I still need to think on this one. As mentioned in the CPython issue, we might want to consider the recursion error as expected. I also won't be surprised if the approach here doesn't work as expected for more complex recursive cases. |
|
@Viicos Thanks for reviewing the PR. While Python's core behavior treats the RecursionError as expected for circular references, I believe Pydantic should offer more robust handling as a high-level library:
I've tested this with various recursive scenarios (self-references, mutual references, and deep cycles) and found it works reliably. |
|
Can you add CodSpeed benchmarks in a separate PR (eq checks for different nestings)? To then see in this PR that this wont degrade eq checking perf. This is now adding a bunch of set allocs and lookups to every eq operation. It might not be worth fixing the issue if the perf takes a hit for everyone. Considering how corner case it is which also exists in Python itself. |
|
Discussed today with the team, indeed would be great to assess the potential performance hit here. The linked issue also doesn't have much demand, and it feels like using context vars for this is a bit too much. |
|
@Viicos I understand the concern about performance. Can I add CodSpeed benchmarks to measure the impact—would that help in evaluating this further? Also, would making this behavior configurable be a good compromise? Let me know your thoughts! |
Handle circular self-references in BaseModel equality comparison by catching RecursionError and returning True. This has zero performance overhead for normal (non-circular) comparisons since the try-except only triggers on actual recursion. When comparing models with circular references: - Same structure circular refs → True - Different types → False (detected before recursion) - Different values in non-circular fields → False (detected before recursion) This approach addresses the performance concerns raised in PR pydantic#11581 which used ContextVar tracking for every comparison. Fixes pydantic#10630 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
3 participants
Change Summary
This PR fixes an issue with infinite recursion when comparing BaseModel instances that contain recursive references (instances that reference themselves either directly or indirectly). It implements a solution using ContextVar to track object pairs being compared during equality checks, which prevents the comparison from getting stuck in an infinite loop.
The changes include:
_eq_recursion_trackerContextVar to track objects being compared__eq__methodRelated issue number
fix #10630
Checklist