-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
bpo-11105: use a lower recursion limit for infinite recursion tests #26550
bpo-11105: use a lower recursion limit for infinite recursion tests #26550
Conversation
@Fidget-Spinner does this patch prevent the crash for your end (do you have resources to test it out?). I highly doubt so (since these functions are giant I don't expect them to be inlined), though if it does then it would be good to know. |
Thanks for working on this! Yup I can test it out, I use MSVC often. Your doubts were right, this is sadly still causing a stack overflow. BTW, testing with buildbots should catch it, the |
Cool! I was worried that they are not available on the public list, but apparently, they are! Thanks for letting me know and testing this out. |
b246c21
to
7f83161
Compare
7f83161
to
d7b496f
Compare
d7b496f
to
f31c841
Compare
🤖 New build scheduled with the buildbot fleet by @isidentical for commit f31c8411cc51590aee9bc8c2b3042911316b546e 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
Seems like the error in
|
I just checked on main branch, this specific test also causes a stack overflow there. So maybe that test failure was previously masked by this one. Your patch fixes the AST stack overflow stack overflow issue for windows. Thanks! Something I found strange is that the exceptions recursion test was added in 4e7a69b but it passed all along. I'm starting to suspect maybe there's some other change elsewhere that caused both |
Ok, bisect tells me that this commit caused
|
f31c841
to
1bf3518
Compare
🤖 New build scheduled with the buildbot fleet by @isidentical for commit 1bf3518db90f3e1c6c8eb1d8daf925e0ae5ee649 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
@isidentical I think if you rebase to the latest 6d518bb, test with buildbots should mostly pass. commit f3fa63e was reverted. |
1bf3518
to
e01e32d
Compare
🤖 New build scheduled with the buildbot fleet by @isidentical for commit e01e32d 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
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.
LGTM. The windows 10 tests are passing. Thanks for your efforts. Without your help I would've taken a lot longer to track down the other windows buildbot failures.
@@ -1999,3 +1999,12 @@ def check_disallow_instantiation(testcase, tp, *args, **kwds): | |||
qualname = f"{name}" | |||
msg = f"cannot create '{re.escape(qualname)}' instances" | |||
testcase.assertRaisesRegex(TypeError, msg, tp, *args, **kwds) | |||
|
|||
@contextlib.contextmanager | |||
def infinite_recursion(max_depth=75): |
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.
Minor suggestion: I think adding a docstring would help others know where/why/when they need this.
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.
Ah, missed this one! Yeap, forgot to do that. Will add a follow up PR.
Thanks @isidentical for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
Thanks @isidentical for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Sorry, @isidentical, I could not cleanly backport this to |
Sorry @isidentical, I had trouble checking out the |
(cherry picked from commit e58d762) Co-authored-by: Batuhan Taskaya <[email protected]>
GH-26605 is a backport of this pull request to the 3.9 branch. |
(cherry picked from commit e58d762) Co-authored-by: Batuhan Taskaya <[email protected]>
GH-26607 is a backport of this pull request to the 3.10 branch. |
https://bugs.python.org/issue11105