Bound the recursion in exception group split and subgroup - #8584
Conversation
Both walk nested groups by calling themselves back through
vm.call_method, and neither pushes a Python frame, so the frame limit
never sees the nesting and the native stack runs out first. A group
nested past the recursion limit segfaults:
e = TypeError(1)
for _ in range(exceeds_recursion_limit()):
e = ExceptionGroup("eg", [e])
e.split(TypeError) # dumps core
vm.with_recursion is the guard for exactly this, the same one comparison
and repr already use, and it turns the crash into the RecursionError
CPython raises. test_deep_split and test_deep_subgroup were skipped on
the segfault and now pass.
Assisted-by: Claude Code:claude-opus-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthrough
ChangesExceptionGroup recursion protection
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change bounds deep exception-group recursion and restores RecursionError behavior, with the reported tests and standard checks passing. No actionable merge-blocking risk remains beyond normal review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_exceptions.py (TODO: 22) dependencies: dependent tests: (no tests depend on exception) Legend:
|
Summary
BaseExceptionGroup.splitand.subgrouprecurse into nested groups throughvm.call_method, which pushes no Python frame.sys.setrecursionlimittherefore never sees the nesting, and a group nested deeply enough runs the native stack out:CPython answers
RecursionError. RustPython dumps core, and so does the test module that covers it:Lib/test/test_exception_group.pysays as much in two skip reasons,TODO: RUSTPYTHON; Segfault, ontest_deep_splitandtest_deep_subgroup. Both are unskipped here.Fix
The two recursive
vm.call_methodcalls now sit insidevm.with_recursion, which is thePy_EnterRecursiveCallcounterpart already used for comparison,repr,__length_hint__and__subclasscheck__. It measures the native stack rather than counting frames, which is the right budget for recursion that never reaches the evaluator.Measurements
Deep case, one process each:
splitat depth 150000RecursionErrorRecursionErrorsubgroupat depth 150000RecursionErrorRecursionErrorShallow behaviour is untouched, checked case by case against CPython:
splitandsubgroupon a flat group, on a nested one, with a predicate instead of a type, with no match at all, and at depth 100 where neither implementation is anywhere near the limit. Every one agrees.test_exception_groupran three times after unskipping, 52 tests, SUCCESS each time, andtest_except_star,test_traceback,test_asyncio.test_taskgroups,test_baseexceptionandtest_exceptionsare green too. Alsocargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi(41 binaries, no failures),cargo testincrates/capi,cargo fmt --check, the clippy invocation from CI, andscripts/check_redundant_patches.pyon the test file.Summary by CodeRabbit