| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I expect this isn't going to make many people happy -- on other platforms, the stack has way more space (I found that 80,000 worked fine on Mac). I recall around Xmas doing an experiment showing that -- but I cannot find the discussion now (maybe it was on Discourse?). |
Sorry, something went wrong.
Agreed. However, the 4000 here is still more than the 1500 that it was prior to #113397 (two weeks ago). And if the goal is to not make it easily possibly to smash the C stack, I think we need to merge something like this. I'd suggest we merge something simple like this for now to restore safety and then consider other options later. Perhaps we can find a way to increase the stack on Windows, or we just have different hard limits on different platforms (but maybe the same defaults). But all that seems to warrant further discussion. |
Sorry, something went wrong.
| with support.infinite_recursion(): | ||
| fib(1250) | ||
|
|
||
| self.assertRaises(RecursionError, fib, 2000) |
There was a problem hiding this comment.
I believe this violates the intention of the test -- it wants to ensure that we can recurse at least a minimum given number of times. Maybe we need both?
Sorry, something went wrong.
There was a problem hiding this comment.
We already have tests for maxing out the recursion limit, both for C and Python recursion, so we probably don't need to add any. But there is no harm in having more tests.
Sorry, something went wrong.
There was a problem hiding this comment.
If we already have those tests, I wonder why they aren't likewise failing... Do you know which those are?
Sorry, something went wrong.
|
Trying to repro this, I still get the same crash in test_functools, after switching to your code. Maybe there's something I should clean up first? I honestly don't know how to get back to a pristine repo (deleting old .obj files etc.) on Windows (on UNIX I'd just use make clobber or make distclean). |
Sorry, something went wrong.
Strange, though I admit I don't develop on Windows much either so I could be doing something wrong here. |
Sorry, something went wrong.
|
Closing as obsolete. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
#113397 recently increased the C recursion limit from 1,500 to 8,000. Unfortunately, this has caused the Windows release buildbots as well as PGO builds to fail with a stack overflow.
The reason, as far as I can tell, is that while there was a set of experiments to determine the largest number that would work for Windows (#113328) (which passed the buildbots), that didn't include the new test that was added in the merged PR #113397 (test_functools:test_lru_recursion) and that new test pushes the stack farther than any of the other tests.
Unfortunately, it doesn't seem to be enough to just limit the test, Py_C_RECURSION_LIMIT also has to be reduced to prevent a C stack overflow.