Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot Recover From StackOverflow in 3.9 Tests #89964

Closed
sweeneyde opened this issue Nov 15, 2021 · 5 comments
Closed

Cannot Recover From StackOverflow in 3.9 Tests #89964

sweeneyde opened this issue Nov 15, 2021 · 5 comments
Labels
3.9 only security fixes OS-windows tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@sweeneyde
Copy link
Member

BPO 45806
Nosy @pfmoore, @db3l, @tjguk, @jkloth, @ambv, @markshannon, @zware, @zooba, @sweeneyde
PRs
  • [3.9] bpo-45806: Fix recovery from stack overflow for 3.9. Again. #29640
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-11-19.18:52:28.209>
    created_at = <Date 2021-11-15.05:40:28.638>
    labels = ['tests', 'OS-windows', 'type-crash', '3.9']
    title = 'Cannot Recover From StackOverflow in 3.9 Tests'
    updated_at = <Date 2022-03-07.22:55:05.832>
    user = 'https://github.com/sweeneyde'

    bugs.python.org fields:

    activity = <Date 2022-03-07.22:55:05.832>
    actor = 'Dennis Sweeney'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-11-19.18:52:28.209>
    closer = 'lukasz.langa'
    components = ['Tests', 'Windows']
    creation = <Date 2021-11-15.05:40:28.638>
    creator = 'Dennis Sweeney'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45806
    keywords = ['patch']
    message_count = 5.0
    messages = ['406339', '406366', '406609', '406610', '414706']
    nosy_count = 9.0
    nosy_names = ['paul.moore', 'db3l', 'tim.golden', 'jkloth', 'lukasz.langa', 'Mark.Shannon', 'zach.ware', 'steve.dower', 'Dennis Sweeney']
    pr_nums = ['29640']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue45806'
    versions = ['Python 3.9']

    @sweeneyde
    Copy link
    Member Author

    In bpo-30570, David Bolen noticed that "py -3.9 -m test test_pickle" consistently crashes on Windows (even though other methods of running that test do not crash, and the test succeeds when failed tests are retried).

    Curiously, it seems that adding using support.infinite_recursion *introduced* the crash rather than preventing it.

    I'm guessing this would have been fixed by #68689, but that fix was rolled back in #69366 for the sake of 3.9 ABI compatibility.

    As of now, 3.9's pycore_ceval.c reads:

        static inline int _Py_RecursionLimitLowerWaterMark(int limit) {
            if (limit > 200) {
                return (limit - 50);
            }
            else {
                return (3 * (limit >> 2));
            }
        }

    But support.infinite_recursion(max_depth=75) has a default 75, leaving a "low-water-mark" of 54, which the recursion apparently never recovers back to in the right way.

    A couple of solutions could fix this:

    (1) Remove the usage of support.infinite_recursion at the test.pickletester.AbstractPickleTests.test_bad_getattr call site.

    (2) Use a different value max_depth. On my machine at least, it seems 75 was a "perfect storm" to cause this issue. Using infinite_recursion(60) or 61 or ... or 74 or 76 or 77 or 78 all pass, but infinite_recursion(75) in particular fails.

    (3) Use fewer calls after the overflow by inlining something like assertRaises:

                 with support.infinite_recursion():
    -                self.assertRaises(RuntimeError, self.dumps, x, proto)
    +                try:
    +                    self.dumps(x, proto)
    +                except RuntimeError:
    +                    pass
    +                else:
    +                    self.fail("RuntimeError not raised")

    (5) Re-visit an ABI-compliant version of #68689, such as #69347

    The output I keep getting without any changes:

    py -3.9 -m test test_pickle -v
    ...
    test_attribute_name_interning (test.test_pickle.CPicklerTests) ... ok
    test_bad_getattr (test.test_pickle.CPicklerTests) ... Fatal Python error: _Py_CheckRecursiveCall: Cannot recover from stack overflow.Python runtime state: initialized

    Current thread 0x000028b0 (most recent call first):
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 3300 in __getattr__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 3300 in __getattr__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 3300 in __getattr__
    ...
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 3300 in __getattr__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 3300 in __getattr__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 3300 in __getattr__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\copyreg.py", line 74 in _reduce_ex
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\test_pickle.py", line 65 in dumps
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\case.py", line 201 in handle
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\case.py", line 739 in assertRaises
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\pickletester.py", line 2381 in test_bad_getattr
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\test\support\init.py", line 1770 in wrapper
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\case.py", line 550 in _callTestMethod
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\case.py", line 592 in run
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\case.py", line 651 in __call__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\suite.py", line 122 in run
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\suite.py", line 84 in __call__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\suite.py", line 122 in run
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\suite.py", line 84 in __call__
    File "C:\Users\sween\Source\Repos\cpython2\39\lib\unittest\suite.py", line 122 in run

    @sweeneyde sweeneyde added 3.9 only security fixes tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump labels Nov 15, 2021
    @jkloth
    Copy link
    Contributor

    jkloth commented Nov 15, 2021

    I'll note that it also fails on first run on the Windows 11 builder:

    https://buildbot.python.org/all/#/builders/737/builds/65

    @ambv
    Copy link
    Contributor

    ambv commented Nov 19, 2021

    New changeset 4296396 by Mark Shannon in branch '3.9':
    [3.9] bpo-45806: Fix recovery from stack overflow for 3.9. Again. (GH-29640)
    4296396

    @ambv
    Copy link
    Contributor

    ambv commented Nov 19, 2021

    Thanks! ✨ 🍰 ✨

    @ambv ambv closed this as completed Nov 19, 2021
    @ambv ambv closed this as completed Nov 19, 2021
    @sweeneyde
    Copy link
    Member Author

    Should this be backported to make the 3.8 Buildbots happy?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes OS-windows tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants