stack overflow protection limit the max recursion depth when using custom frame evaluation

[art049]

Bug report

Enabling the perf trampoline messes with the recursion limit.

Reproduction

def recurse(n):
    if n == 0:
        return
    recurse(n - 1)


if __name__ == "__main__":
    import sys

    n = 801
    sys.setrecursionlimit(n)
    recurse(n - 2)

The following execution works perfectly:

python recurse.py

However, this gives a recursion error:

python -X perf recurse.py

Traceback (most recent call last):
  File "<...>/recurse.py", line 12, in <module>
    recurse(n - 2)
  File "<...>/recurse.py", line 4, in recurse
    recurse(n - 1)
  File "<...>/recurse.py", line 4, in recurse
    recurse(n - 1)
  File "<...>/recurse.py", line 4, in recurse
    recurse(n - 1)
  [Previous line repeated 796 more times]
RecursionError: maximum recursion depth exceeded

Strangely, the issue only arises when n >= 801 and not under this value.

@pablogsal if you have an idea of what could create the problem, I'd be happy to work on a fix!

Your environment

• CPython versions tested on: 3.12.0b1, 3.12.0-dev(2c02c68)
• OS: linux 5.15

[art049]

The problem seems to come from the C recursion limit added in #91079

This explains why the behavior happens only after a depth of 800:

cpython/Include/cpython/pystate.h

Line 258 in 2c02c68

# define C_RECURSION_LIMIT 800

The custom frame eval function set by the perf trampoline indirectly decreases tstate->c_recursion_remaining, changing the max recursion depth.

I'm not sure yet if it's specifically related to the py_trampoline_evaluator or if the problem appears more globally when a custom frame eval is used.
Edit: I managed to reproduce the issue as well with any other custom eval frame functions.

[sunmy2019]

Confirmed it's restricted by the C recursion limit.

I think the correct thing here is to expose an API to change this. But this sound a little more, and may not be backported to 3.12? @markshannon @gpshead

    int py_recursion_remaining;
    int py_recursion_limit;

    int c_recursion_remaining;
+    int c_recursion_limit;

[art049]

I think the correct thing here is to expose an API to change this. But this sound a little more, and may not be backported to 3.12?

It might still break some existing codebases that were using a custom eval function, limiting their recursion depth to 800 without AFAIK any possible workaround.

[pablogsal]

It might still break some existing codebases that were using a custom eval function, limiting their recursion depth to 800 without AFAIK any possible workaround.

Using a custom eval function and perf is not supported because perf support relies on a custom eval function. This means that unless you wrap it you are out of luck.

[art049]

Using a custom eval function and perf is not supported because perf support relies on a custom eval function. This means that unless you wrap it you are out of luck.

Yes totally. I wasn't very clear but I also managed to reproduce the issue with another dummy custom eval function like this:

static PyObject * dummy_custom_evaluator(PyThreadState *ts, _PyInterpreterFrame *frame, int throw)
{
    return _PyEval_EvalFrameDefault(ts, frame, throw);
}

Thus for existing uses of the custom eval function, it might break existing implementations.
Since this issue is also affecting, the perf support, I'm not sure if I should create another issue.

[pablogsal]

Using a custom eval function and perf is not supported because perf support relies on a custom eval function. This means that unless you wrap it you are out of luck.

Yes totally. I wasn't very clear but I also managed to reproduce the issue with another dummy custom eval function like this:

static PyObject * dummy_custom_evaluator(PyThreadState *ts, _PyInterpreterFrame *frame, int throw)
{
    return _PyEval_EvalFrameDefault(ts, frame, throw);
}

Thus for existing uses of the custom eval function, it might break existing implementations. Since this issue is also affecting, the perf support, I'm not sure if I should create another issue.

I think we should just use this issue and remove the perf-specific part

[art049]

Is this the expected behavior from the stack overflow protection put in place in #91079? (cc @markshannon)
I'd be up working on a fix regarding this but do you think the behavior should indeed be changed?

I managed a dirty workaround incrementing the c_recursion_remaining variable in the custom frame evaluator but it's probably not what we want to do globally.