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

Jump to bottom

test_bad_getattr crashes on APPX test #84638

Closed
zooba opened this issue Apr 30, 2020 · 8 comments
Closed

test_bad_getattr crashes on APPX test #84638

zooba opened this issue Apr 30, 2020 · 8 comments
Assignees
@zooba
Labels
3.8 only security fixes 3.9 only security fixes OS-windows type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@zooba
Copy link
Member

zooba commented Apr 30, 2020

BPO 40458
Nosy @pfmoore, @vstinner, @tjguk, @ambv, @zware, @zooba, @miss-islington
PRs
  • bpo-40458: Increase reserved stack space to prevent overflow crash on Windows #19845
  • [3.8] bpo-40458: Increase reserved stack space to prevent overflow crash on Windows (GH-19845) #19941
    		•

    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 = 'https://github.com/zooba'
closed_at = <Date 2020-09-24.20:39:51.631>
created_at = <Date 2020-04-30.23:10:27.627>
labels = ['3.8', 'OS-windows', 'type-crash', '3.9']
title = 'test_bad_getattr crashes on APPX test'
updated_at = <Date 2020-09-24.20:39:51.631>
user = 'https://github.com/zooba'

    bugs.python.org fields:

    activity = <Date 2020-09-24.20:39:51.631>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2020-09-24.20:39:51.631>
closer = 'steve.dower'
components = ['Windows']
creation = <Date 2020-04-30.23:10:27.627>
creator = 'steve.dower'
dependencies = []
files = []
hgrepos = []
issue_num = 40458
keywords = ['patch']
message_count = 8.0
messages = ['367803', '367880', '367881', '367888', '368060', '368185', '368186', '368189']
nosy_count = 7.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'lukasz.langa', 'zach.ware', 'steve.dower', 'miss-islington']
pr_nums = ['19845', '19941']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue40458'
versions = ['Python 3.8', 'Python 3.9']
    @zooba
    Copy link
    Member Author

    zooba commented Apr 30, 2020

    The Windows CI machines on Azure Pipelines run additional tests to check an "installed" layout and with the UWP entry point that's used for the Windows Store package.

    These tests have been failing intermittently (though regularly) with a stack overflow crash in the PyPickler tests.

    Example: https://dev.azure.com/Python/cpython/_build/results?buildId=62055&view=results

    test_attribute_name_interning (test.test_pickle.PyPicklerTests) ... ok
    File "D:\a\1\b\layout-appx-amd64\lib\test\pickletester.py", line 3085 in __getattr__
    File "D:\a\1\b\layout-appx-amd64\lib\test\pickletester.py", line 3085 in __getattr__
    File "D:\a\1\b\layout-appx-amd64\lib\test\pickletester.py", line 3085 in __getattr__
    File "D:\a\1\b\layout-appx-amd64\lib\test\pickletester.py", line 3085 in __getattr__
    ...

    I assume this is due to having more code on the start at the start, and so the recursion limit isn't low enough. But it might also be worth checking this particular case to see whether there is unnecessary data being kept on the stack (e.g. in local C variables).

    The crash occurs in both 3.8 and master, but not 3.7.

    @zooba zooba added 3.8 only security fixes 3.9 only security fixes OS-windows labels Apr 30, 2020
    @zooba
    Copy link
    Member Author

    zooba commented May 1, 2020

    Turns out the stack reservation was different, which is why it was crashing.

    As part of diagnosing it, I added the recursion count to faulthandler's output on Windows, but couldn't see if/where to do it for other platforms - Victor, any suggestions?

    @zooba zooba added type-crash A hard crash of the interpreter, possibly with a core dump labels May 1, 2020
    @zooba
    Copy link
    Member Author

    zooba commented May 1, 2020

    Also, it was really test_bad_getattr that was crashing.

    @zooba zooba changed the title test_attribute_name_interning crashes on APPX test test_bad_getattr crashes on APPX test May 1, 2020
    @zooba zooba changed the title test_attribute_name_interning crashes on APPX test test_bad_getattr crashes on APPX test May 1, 2020
    @vstinner
    Copy link
    Member

    vstinner commented May 1, 2020

    Also, it was really test_bad_getattr that was crashing.

    Ah, an old friend. It seems like an issue with the maximum stack size. Here are my notes:
    https://pythondev.readthedocs.io/unstable_tests.html#unlimited-recursion

    Either reduce Python maximum stack depth, or increase the C stack size.

    @zooba
    Copy link
    Member Author

    zooba commented May 4, 2020

    Yeah, I already got that part. If you check the PR, I added some better diagnostics to faulthandler for this case, but I don't see where I can add it for other platforms?

    @zooba
    Copy link
    Member Author

    zooba commented May 5, 2020

    New changeset ac4bf42 by Steve Dower in branch 'master':
    bpo-40458: Increase reserved stack space to prevent overflow crash on Windows (GH-19845)
    ac4bf42

    @zooba
    Copy link
    Member Author

    zooba commented May 5, 2020

    Merged without the improved diagnostics. I might add that later if I get time.

    Łukasz - this is a fairly trivial crash fix that'd be nice (and safe) to pull into 3.8.3, but not critical. Your call.

    @miss-islington
    Copy link
    Contributor

    New changeset a6a116c by Miss Islington (bot) in branch '3.8':
    bpo-40458: Increase reserved stack space to prevent overflow crash on Windows (GH-19845)
    a6a116c

    @zooba zooba closed this as completed Sep 24, 2020
    @zooba zooba closed this as completed Sep 24, 2020
    @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
    Assignees

    @zooba zooba

    Labels
    3.8 only security fixes 3.9 only security fixes OS-windows type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @zooba @miss-islington