Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manage recursion limit preventing RuntimeError #68

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jayvdb
Copy link
Member

@jayvdb jayvdb commented May 6, 2016

pyflakes has traditionally recursed with a handler for every
level of the ast. The ast depth can become very large, especially
for an expression containing many binary operators.

Python has a maximum recursion limit, defaulting to a low number
like 1000, which resulted in a RuntimeError for the ast of:

x = 1 + 2 + 3 + ... + 1001

To workaround this problem, pyflakes now increases the recursion
limit at runtime when it knows it will be exceeded.

Fixes lp:1507827

@jayvdb
Copy link
Member Author

jayvdb commented May 6, 2016

I've gone for the simple workaround for https://bugs.launchpad.net/pyflakes/+bug/1507827 , as this is the only Exception I can find, and the alternative (flattening) is a complex patch not suitable for a patch or even minor release.

I'd really like this solved, as it means I can run pyflakes . in a parent directory above all of my python packages as a form of regression testing, but that fails currently due to this exception.

I have one more minor bug in my flatten patch, but should have it up in a few hours. It is easy enough to verify for unary/boolean/binary ops only, but the bug would still exist elsewhere. Solving it for boolean/binary op is not easy, as boolean/binary ops can contain lambda's, and pyflakes has deferred processing of lambdas. I've fixed that problem - my issue is now just generators on py2.7 only. What I do know is the fix is going to need a lot of vetting & testing before it is released.

@jayvdb
Copy link
Member Author

jayvdb commented May 7, 2016

#70 is the companion patch, which reduces the recursion so it doesnt hit the limit easily. It still can hit the limit for extremely crazy code, but it is quite improbable.

I'm going to slightly refactor this patch (68) so it is less of a performance hit, so it can be merged into the 1.2. or 1.x series promptly.


def test_recursion_limit(self):
# Using self._recursionlimit * 10 tends to cause CPython to core dump.
r = range(self._recursionlimit * 9)
Copy link
Member Author

Choose a reason for hiding this comment

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

Lots of segmentation fault now after a rebase, on self._recursionlimit * 9

I'll try reducing it.

@jayvdb jayvdb force-pushed the manage_recursion_limit branch 2 times, most recently from 775c200 to e2488ff Compare July 11, 2018 16:05
pyflakes has traditionally recursed with a handler for every
level of the ast.  The ast depth can become very large, especially
for an expression containing many binary operators.

Python has a maximum recursion limit, defaulting to a low number
like 1000, which resulted in a RuntimeError for the ast of:

    x = 1 + 2 + 3 + ... + 1001

To workaround this problem, pyflakes now increases the recursion
limit at runtime when it knows it will be exceeded.

Fixes lp:1507827
@jayvdb
Copy link
Member Author

jayvdb commented Jul 11, 2018

The test is failing on older versions of PyPy only.
Leaving this open as it does work, and is slightly faster than #70 .
If it is preferred, we could skip the tests on those environments where pushing the runtime beyond the limit causes core dumps.

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