Skip to content

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

Open
Open
stack overflow protection limit the max recursion depth when using custom frame evaluation#105003
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@art049
art049
opened on May 26, 2023

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

Activity

  1. art049
    added
    type-bugAn unexpected behavior, bug, or error
    on May 26, 2023

  2. art049 commented on May 27, 2023

    @art049
    art049
    on May 27, 2023
    ContributorAuthor

    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.

  3. art049
    changed the title [-]stack trampoline changes the max recursion depth[/-] [+]perf trampoline changes the max recursion depth[/+] on May 27, 2023

  4. sunmy2019 commented on May 27, 2023

    @sunmy2019
    sunmy2019
    on May 27, 2023
    Member

    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;

  5. art049 commented on May 27, 2023

    @art049
    art049
    on May 27, 2023
    ContributorAuthor

    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.

  6. pablogsal commented on May 27, 2023

    @pablogsal
    pablogsal
    on May 27, 2023
    Member

    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.

  7. art049 commented on May 28, 2023

    @art049
    art049
    on May 28, 2023
    ContributorAuthor

    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.

  8. art049
    changed the title [-]perf trampoline changes the max recursion depth[/-] [+]Custom frame evaluation (and perf trampoline) changes the max recursion depth[/+] on May 28, 2023

  9. pablogsal commented on May 28, 2023

    @pablogsal
    pablogsal
    on May 28, 2023
    Member

    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

  10. art049
    changed the title [-]Custom frame evaluation (and perf trampoline) changes the max recursion depth[/-] [+]stack overflow protection limit the max recursion depth when using custom frame evaluation[/+] on May 28, 2023

  11. art049 commented on Jun 8, 2023

    @art049
    art049
    on Jun 8, 2023
    ContributorAuthor

    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.

  12. iritkatriel
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions