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

import in finally results in SystemError #75467

Closed
tacaswell mannequin opened this issue Aug 27, 2017 · 7 comments
Closed

import in finally results in SystemError #75467

tacaswell mannequin opened this issue Aug 27, 2017 · 7 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@tacaswell
Copy link
Mannequin

tacaswell mannequin commented Aug 27, 2017

BPO 31286
Nosy @brettcannon, @ncoghlan, @ericsnowcurrently, @serhiy-storchaka, @tacaswell
PRs
  • bpo-31286: Fixed stack usage in absolute imports with binding a submo… #3217
    		•

    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 2017-08-29.12:50:01.846>
created_at = <Date 2017-08-27.02:38:08.893>
labels = ['interpreter-core', '3.7', 'type-crash']
title = 'import in finally results in SystemError'
updated_at = <Date 2017-08-29.13:18:53.258>
user = 'https://github.com/tacaswell'

    bugs.python.org fields:

    activity = <Date 2017-08-29.13:18:53.258>
actor = 'tcaswell'
assignee = 'none'
closed = True
closed_date = <Date 2017-08-29.12:50:01.846>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2017-08-27.02:38:08.893>
creator = 'tcaswell'
dependencies = []
files = []
hgrepos = []
issue_num = 31286
keywords = []
message_count = 7.0
messages = ['300911', '300913', '300914', '300915', '300984', '300985', '300988']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'ncoghlan', 'eric.snow', 'serhiy.storchaka', 'tcaswell']
pr_nums = ['3217']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue31286'
versions = ['Python 3.7']
    @tacaswell
    Copy link
    Mannequin Author

    tacaswell mannequin commented Aug 27, 2017

    Code to reproduce:

    def import_in_finally_fail():
    try:
        print('yo')
    finally:
        import asyncio.queues as aq

    Results in:

    In [68]: import_in_finally_fail()
    yo
    ---------------------------------------------------------------------------

    SystemError                               Traceback (most recent call last)
<ipython-input-68-4a6d2efcb22b> in <module>()
----> 1 import_in_finally_fail()

    /tmp/py2661VBj in import_in_finally_fail()

    SystemError: 'finally' pops bad exception

    The exact import does matter, but it needs to be at least 2 modules deep and be aliased.

    By patching cpython to put what finally pops off the stack in the error message it looks like it is the parent module of the import.

    This was found via the Matplotlib test suite (ex https://travis-ci.org/matplotlib/matplotlib/jobs/262907186 ) and reported to pytest (pytest-dev/pytest#2674)

    @tacaswell tacaswell mannequin added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Aug 27, 2017
    @serhiy-storchaka
    Copy link
    Member

    Confirmed. The bug is 3.7 only.

    @serhiy-storchaka
    Copy link
    Member

    The regression was introduced in bpo-30024. It was missed a difference between LOAD_ATTR and IMPORT_FROM. LOAD_ATTR replaces the object with the value of the attribute (do not changing the stack pointer). IMPORT_FROM keeps the original module on the stack and pushes the value of the attribute increasing the stack pointer.

    There are two ways of fixing this issue.

    1. Add two instructions ROT_TWO and POP_TOP after every IMPORT_FROM in the bytecode generated from "import a.b.c as d".

    2. Make IMPORT_FROM popping the original module from the stack. The explicit DUP_TOP instruction should be inserted before every IMPORT_FROM in the bytecode generated from "from a import b, c".

    @serhiy-storchaka
    Copy link
    Member

    PR 3217 implements the first way. It also includes some refactoring. Remove all pyc-files before testing.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 265fcc5 by Serhiy Storchaka in branch 'master':
    bpo-31286, bpo-30024: Fixed stack usage in absolute imports with (bpo-3217)
    265fcc5

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your report Thomas!

    Don't forget to remove pyc-files before re-running the Matplotlib test suite.

    @tacaswell
    Copy link
    Mannequin Author

    tacaswell mannequin commented Aug 29, 2017

    Your welcome!

    Matplotlib ended up just moving the import out of the finally block once we
    understood the issue.

    Tom

    On Tue, Aug 29, 2017 at 8:50 AM Serhiy Storchaka <report@bugs.python.org>
    wrote:

    Serhiy Storchaka added the comment:

    Thank you for your report Thomas!

    Don't forget to remove pyc-files before re-running the Matplotlib test
    suite.

    ----------
    resolution: -> fixed
    stage: patch review -> resolved
    status: open -> closed

    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue31286\>

    @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
    No one assigned
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) 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
    1 participant
    @serhiy-storchaka