Skip to content

Add RecursionError - #1477

Merged
windelbouwman merged 7 commits into
RustPython:masterfrom
lazylife7157:recursion_error
Oct 6, 2019
Merged

windelbouwman merged 7 commits into
RustPython:masterfrom
lazylife7157:recursion_error

Conversation

@lazylife7157

@lazylife7157 lazylife7157 commented Oct 5, 2019 •

Copy link
Copy Markdown
Contributor

Partial solution of #1374

I made a small change to check recursion depth before push the frame and then raise error instead of run until segmentation fault occurs.

But it seems there is still a problem. RustPython occurs segmentation fault on 700 (debug) ~ 2600 (release) recursive calls while cpython is fine until 15000. This is the reason that I set default limit lower than cpython.

If there is any inappropriate changes, please let me know. Thank you.

@coolreader18 coolreader18 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks for contributing!

Comment thread tests/snippets/sysmod.py Outdated
try:
recursive_call(300)
except RecursionError as exc:
pass

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use with assert_raises(RecursionError): ... here.

Comment thread vm/src/vm.rs Outdated
pub use_tracing: RefCell<bool>,
pub signal_handlers: RefCell<[PyObjectRef; NSIG]>,
pub settings: PySettings,
pub recursion_limit: RefCell<usize>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be a Cell<usize>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's much better! Thanks.

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.

3 participants