gh-113655: Reduce recursion limit to a safe number for Windows #113668
Conversation
mdboom
added
the
🔨 test-with-buildbots
bedevere-bot
removed
the
🔨 test-with-buildbots
mdboom
added
OS-windows
🔨 test-with-buildbots
bedevere-bot
removed
the
🔨 test-with-buildbots
|
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?). |
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. |
| @@ -1876,8 +1876,7 @@ def fib(n): | |||
|
|
|||
| if not support.Py_DEBUG: | |||
| 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?
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.
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?
|
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 |
Strange, though I admit I don't develop on Windows much either so I could be doing something wrong here. |
#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_LIMITalso has to be reduced to prevent a C stack overflow.