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

Crash when modifying sys.modules during import #59783

Closed
Yhg1s opened this issue Aug 7, 2012 · 6 comments
Closed

Crash when modifying sys.modules during import #59783

Yhg1s opened this issue Aug 7, 2012 · 6 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@Yhg1s
Copy link
Member

Yhg1s commented Aug 7, 2012

BPO 15578
Nosy @Yhg1s, @gpshead, @bitdancer, @ericsnowcurrently
Files
  • eric.snow_issue15578.diff
  • 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/ericsnowcurrently'
    closed_at = <Date 2016-09-08.02:10:06.271>
    created_at = <Date 2012-08-07.20:50:29.778>
    labels = ['interpreter-core', 'type-crash']
    title = 'Crash when modifying sys.modules during import'
    updated_at = <Date 2016-09-08.05:26:28.758>
    user = 'https://github.com/Yhg1s'

    bugs.python.org fields:

    activity = <Date 2016-09-08.05:26:28.758>
    actor = 'python-dev'
    assignee = 'eric.snow'
    closed = True
    closed_date = <Date 2016-09-08.02:10:06.271>
    closer = 'eric.snow'
    components = ['Interpreter Core']
    creation = <Date 2012-08-07.20:50:29.778>
    creator = 'twouters'
    dependencies = []
    files = ['26883']
    hgrepos = []
    issue_num = 15578
    keywords = ['patch']
    message_count = 6.0
    messages = ['167640', '167642', '168495', '185285', '274953', '274965']
    nosy_count = 6.0
    nosy_names = ['twouters', 'gregory.p.smith', 'r.david.murray', 'python-dev', 'eric.snow', 'nordaux']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue15578'
    versions = ['Python 2.7']

    @Yhg1s
    Copy link
    Member Author

    Yhg1s commented Aug 7, 2012

    In a submodule of a package, replacing the parent package in sys.modules during import of the package (meaning the only reference to it is in sys.modules) and then importing itself (or something from itself) crashes the interpreter:

    centurion:/python/crasher > cat sa/init.py
    import v1
    centurion:
    /python/crasher > cat sa/v1/init.py
    import sys
    sys.modules['sa'] = sys.modules[name]
    import sa
    centurion:~/python/crasher > python -c 'import sa'
    Segmentation fault (core dumped)

    It seems the crash is not entirely deterministic, as we've had the original code this was reduced from run in Python 2.4 and 2.6 for years, and only discovered the crash when switching to 2.7. However, running the reduced testcase crashes for me in all of Python 2.7, 2.6, 2.4 and 2.2, in debug builds and opt builds. Given the nature of the bug I expect debug builds to crash reliably.

    I haven't found the actual source of the problem, but what seems to happen is that the import mechanism has a borrowed reference to the 'sa' module around, assuming it'll stay alive while the submodules are imported because it's also stored in sys.modules. This assumption is incorrect. However, changing the import in sa/init.py into an absolute or explicit relative import seems to fix the problem, which means this probably won't affect Python 3.

    @Yhg1s Yhg1s added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Aug 7, 2012
    @bitdancer
    Copy link
    Member

    If I remember correctly there were non-trivial improvements made to the way modules are finalized in 2.7 compared to earlier versions. That could be what exposed the bug. The pre-2.7 code might have been leaving another reference alive because of your more complex real world example, whereas the simplified example doesn't.

    @ericsnowcurrently
    Copy link
    Member

    Here's the deal. import_module_level() gets called for v1 from sa (where "globals" comes from). In that function it first calls get_parent(), which returns a borrowed reference to the sa module object. Then that parent object is passed to load_next() where the actual load of v1 will take place (and the segfault happens).

    The problem is that get_parent() returns a borrowed reference. When the sa module is replaced in sys.modules, the old sa module is decref'ed. That's fine except load_next is using that same module as the parent. Enter segfault, stage left.

    Here's a quick patch that fixes the failure (along with a test).

    @brettcannon
    Copy link
    Member

    Eric has commit rights now so he can commit his own fix. =)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 8, 2016

    New changeset 2d6dd8402d77 by Eric Snow in branch '2.7':
    Issue bpo-15578: Correctly incref the parent module while importing.
    https://hg.python.org/cpython/rev/2d6dd8402d77

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 8, 2016

    New changeset 731e5617cc8d by Gregory P. Smith in branch '2.7':
    Fix placement of Misc/NEWS item for issue bpo-15578.
    https://hg.python.org/cpython/rev/731e5617cc8d

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants