3.12 setrecursionlimit is ignored in connection with @functools.cache
#112215
Comments
|
also tested on Linux (with the call to As well, there is no discrepancy with 3.11 vs 3.12 if The current master is bad too. In my testing I modified the lines I removed the 1st one (it's very recent), and only left |
|
I found the 1st bad commit, 7644935, by bisecting: |
|
@markshannon - greetings from Oxford, by the way :-) |
|
Yes. I was suggesting exposing an API for the C recursion limit, but not receiving active feedback. See the discussion here: #107263 |
|
@PeterLuschny - basic Fibonacci # fib.py
import sys
sys.setrecursionlimit(2000)
from functools import cache
@cache
def fib(n):
if n<1: return 0
if n==1: return 1
return fib(n-1) + fib(n-2)
print(fib(500))shows the bug just as well |
|
it's not "probably in connection with |
PeterLuschny
changed the title
Eclips4
changed the title
@functools.cache
|
The bug is triggered by the use of the C implementation of --- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -641,7 +641,7 @@ def cache_clear():
return wrapper
try:
- from _functools import _lru_cache_wrapper
+ from _functools import no_no_lru_cache_wrapper
except ImportError:
passand see In a way, it's to be expected, as C code isn't governed by |
|
@arhadthedev This is not related to performance. It's a new hard-coded limit not adjustable. |
|
@markshannon ^^ |
|
We added the C recursion limit for safety reasons. We don't want to give that up. We also opted for portability of code by setting the C recursion limit to a single value for all platforms. |
|
I don't understand how you can nuke all the recursive uses of Python C extensions in the name of "safety", sorry. For a small sampling, have a look how many code chunks on GitHub have |
|
@dimpase: your tone is aggressive. that's uncalled for. please keep it civil. |
Apologies. In my defense, I can only say that I am speaking from one of the pitfalls wisely predicted by the SC here. in the rejection of PEP 651 – Robust Stack Overflow Handling |
I agree with this. It causes large inconvenience for users who do end up relying on recursion. Their right to use C recursion is deprived, even without notice. |
|
And, by the way, limiting the C stack cannot prevent all stack overflows. 500, 1000, or 1500 are just rough estimates. We can still create stack overflows under these conditions. The idiomatic way is to register a signal handler (on UNIX) / VectoredExceptionHandler (on Windows) to check if the stack pointer is out of the boundary while not recoverable. I am fine with having these limits, but please provide a way to circumvent them. Why are users forbidden to use large stack memory even if they provide one? |
|
I think it would be more productive to come up with a way that we can calculate the C stack size accurately rather than continue to explain why it's not great (we know, we know). |
Yes, we can estimate it better. We don't need to be that accurate. We can make sure the user can make it larger. For example on Linux, we can query the stack limit at the When the user sets the stack limit of 1MB, he gets When he needs 2000, all he needs to do is set the stack limit to |
|
|
|
Note: 13 of the 14 tests named above still pass in a debug build. As I recall, |
|
Hm, maybe the CI uses debug build. |
|
Marking this as release blocker since we discussed this off-line. The plan is to restore 3.12 to the 3.11 behavior (which is a little tricky since various tests were added/updated, but it's doable). For 3.13 we will do per-platform things (and per-config-setting, e.g. debug) by default and add an API to change the C recursion limit. See also #113403 (comment) for some measurements of what we can actually support without crashing (on Mac, but apparently Linux is similar, and Windows is dramatically different). |
I get this too. Windows 11, main branch. We also see it in the Azure Pipelines CI build, e.g. |
|
What's the current state of this issue? I know some of the test failures were worked around, but do we need anything to address user concerns that hasn't already been done? Should this block today's 3.12 release? |
|
Do you need to delegate the reverts to a developer-in-residence? |
|
#115083 ports the various C recursion limit changes from main, but it is unlikely to be ready until tomorrow as I need to get the limits and tests fixed for wasi. I don't think it should block the release. |
|
@guido said:
Do I understand correctly that the plan for 3.12 changed to backporting various changes from main? |
|
Is there anything left to do for this? A fix went in for the next 3.12 (scheduled in two months). Is there anything that still needs to be done for 3.13, before or after today's 3.13 alpha 4? |
|
Like Mark, I don't think this issue should block alpha 4. |
|
We might change the implementation for 3.13, to use the actual stack depth (in bytes) rather than a count. |
On Python 3.12, this provokes a stack overflow in the scheduler. It is
not quite clear why that's the case; pure-Python recursion even with
generators seems to respond well to setrecursionlimit():
```py
def f(n):
if n:
yield from f(n-1)
else:
yield 5
import sys
sys.setrecursionlimit(3500)
print(list(f(3400)))
```
That said, there have been [behavior](python/cpython#96510)
[changes](python/cpython#112215)
in Py3.12 in this regard, but it is not clear what exactly
about Loopy's behavior makes it fall into the 'bad' case.
On Python 3.12, this provokes a stack overflow in the scheduler. It is
not quite clear why that's the case; pure-Python recursion even with
generators seems to respond well to setrecursionlimit():
```py
def f(n):
if n:
yield from f(n-1)
else:
yield 5
import sys
sys.setrecursionlimit(3500)
print(list(f(3400)))
```
That said, there have been [behavior](python/cpython#96510)
[changes](python/cpython#112215)
in Py3.12 in this regard, but it is not clear what exactly
about Loopy's behavior makes it fall into the 'bad' case.
On Python 3.12, this provokes a stack overflow in the scheduler. It is
not quite clear why that's the case; pure-Python recursion even with
generators seems to respond well to setrecursionlimit():
```py
def f(n):
if n:
yield from f(n-1)
else:
yield 5
import sys
sys.setrecursionlimit(3500)
print(list(f(3400)))
```
That said, there have been [behavior](python/cpython#96510)
[changes](python/cpython#112215)
in Py3.12 in this regard, but it is not clear what exactly
about Loopy's behavior makes it fall into the 'bad' case.
|
I cannot reproduce error from #127346 on my macos, so it's probably a Windows issue? |
|
beware that C stack size issues are typically local platform & build config dependent as to what numbers you need to use to bump into a given issue. |
Bug report
A minimal example:
CPython versions tested on:
CPython main branch
Operating systems tested on:
Windows
Edit: This (simpler) example is from @dimpase .
Linked PRs
The text was updated successfully, but these errors were encountered: