GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add RecursionError by lazylife7157 · Pull Request #1477 · RustPython/RustPython

Conversation

@lazylife7157

@lazylife7157 lazylife7157 commented Oct 5, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
coolreader18, windelbouwman, and j30ng reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
coolreader18
coolreader18 approved these changes Oct 5, 2019
View reviewed changes

@coolreader18 coolreader18 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/snippets/sysmod.py Outdated
try:
recursive_call(300)
except RecursionError as exc:
pass

@coolreader18 coolreader18 Oct 5, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
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>,

@coolreader18 coolreader18 Oct 5, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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>

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@lazylife7157 lazylife7157 Oct 5, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
lazylife7157 added 7 commits October 6, 2019 11:31
@lazylife7157
Add RecursionError
c90ffa7
@lazylife7157
Add recursion limit test
87d31e4
@lazylife7157
Raise RecursionError when maximum recursion depth exceeded
297777a
@lazylife7157
Add sys.getrecursionlimit && sys.setrecursionlimit
c15c428
@lazylife7157
Refine recursion test
c80c68c
@lazylife7157
Change type of recursion_limit to Cell
540bcbc
@lazylife7157
Formatting
d0d5dbe
@windelbouwman
windelbouwman merged commit c2118a6 into RustPython:master Oct 6, 2019
@DimitrisJim DimitrisJim mentioned this pull request Aug 5, 2021
exec() crashes Rust Python when taking large expression as argument #2783
Closed
@DimitrisJim DimitrisJim mentioned this pull request Aug 24, 2021
Bump recursion limit to match CPython's #2959
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coolreader18 coolreader18 coolreader18 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@lazylife7157 @coolreader18 @windelbouwman
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.