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

[subinterpreters] atexit_callfuncs() crashing within Py_Finalize() when using multiple interpreters. #50780

Closed
grahamd mannequin opened this issue Jul 21, 2009 · 15 comments
Labels
topic-subinterpreters type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@grahamd
Copy link
Mannequin

grahamd mannequin commented Jul 21, 2009

BPO 6531
Nosy @loewis, @terryjreedy, @ncoghlan, @pitrou, @bitdancer, @encukou, @ericsnowcurrently, @Dormouse759
Files
  • atexitsubinterps.patch
  • 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 2018-09-10.16:42:56.216>
    created_at = <Date 2009-07-21.11:19:28.383>
    labels = ['expert-subinterpreters', 'type-crash']
    title = '[subinterpreters] atexit_callfuncs() crashing within Py_Finalize() when using multiple interpreters.'
    updated_at = <Date 2020-05-15.01:13:47.316>
    user = 'https://bugs.python.org/grahamd'

    bugs.python.org fields:

    activity = <Date 2020-05-15.01:13:47.316>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-09-10.16:42:56.216>
    closer = 'petr.viktorin'
    components = ['Subinterpreters']
    creation = <Date 2009-07-21.11:19:28.383>
    creator = 'grahamd'
    dependencies = []
    files = ['24268']
    hgrepos = []
    issue_num = 6531
    keywords = ['patch']
    message_count = 15.0
    messages = ['90753', '90757', '90758', '90759', '139225', '139231', '151395', '151396', '151494', '151495', '151496', '151527', '151531', '307702', '308057']
    nosy_count = 11.0
    nosy_names = ['loewis', 'terry.reedy', 'ncoghlan', 'pitrou', 'grahamd', 'r.david.murray', 'petr.viktorin', 'python-dev', 'eric.snow', 'chn', 'Dormouse759']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue6531'
    versions = ['Python 3.5', 'Python 3.6']

    @grahamd
    Copy link
    Mannequin Author

    grahamd mannequin commented Jul 21, 2009

    I am seeing a crash within Py_Finalize() with Python 3.0 in mod_wsgi. It looks like the
    patches for issue-4200 were not adequate and that this wasn't picked up at the time.

    This new problem I am seeing looks like it may be linked to where the 'atexit' module is
    initialised/imported in a sub interpreter but never imported in the main interpreter. I can
    avoid the crash by having:

        PyImport_ImportModule("atexit");
    
        Py_Finalize();

    At a guess, the problem is because in atexit_callfuncs():

        module = PyState_FindModule(&atexitmodule);
        if (module == NULL)
            return;

    still returns a module for case where imported in a sub interpreter but not in main
    interpreter, so doesn't return, but then code which follows:

        modstate = GET_ATEXIT_STATE(module);
    if (modstate->ncallbacks == 0)
        return;
    

    returns NULL for modstate for the main interpreter as PyInit_atexit() had never been called
    for the main interpreter as the 'atexit' module was never imported within that interpreter.

    The fix would appear to be to check modstate for being NULL and return. Ie.,

        module = PyState_FindModule(&atexitmodule);
        if (module == NULL)
            return;
        modstate = GET_ATEXIT_STATE(module);
    if (modstate == NULL)
        return;
    
    if (modstate->ncallbacks == 0)
        return;
    

    The only thing I am uncertain about is why PyState_FindModule() would return an object. I
    cant find any documentation about that function so not entirely sure what it is meant to do.
    I would have thought it would be returning data specific to the interpreter, but if never
    imported in that interpreter, why would there still be an object recorded.

    BTW, I have marked this as for Python 3.1 as well, but haven't tested it on that. The code in
    'atexit' module doesn't appear to have changed though so assuming it will die there as well.

    For now am using the workaround in mod_wsgi.

    @grahamd grahamd mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 21, 2009
    @bitdancer
    Copy link
    Member

    I strongly recommend you move to 3.1. 3.0 will not be getting any more
    bug patches.

    @grahamd
    Copy link
    Mannequin Author

    grahamd mannequin commented Jul 21, 2009

    As a provider of software that others use I am just making mod_wsgi usable
    with everything so users can use whatever they want. You telling me to use
    Python 3.1 isn't going to stop people from using Python 3.0 if that is
    what they happen to have installed. Just look at how many people still use
    really old Python 2.X versions. Ultimately I don't care which Python
    version it is fixed in as I have the work around anyway.

    @bitdancer
    Copy link
    Member

    I'm sorry, I wasn't clear. We are recommending that no one use Python
    3.0 for production. Python 3.1 fixed some significant performance
    issues. We are hoping that distribution packagers will not ship 3.0,
    but will ship 3.1 instead. But if you want to support 3.0, that's a
    fine thing.

    I should have just kept my mouth shut, especially since I don't have any
    input on the bug itself ;)

    @terryjreedy
    Copy link
    Member

    What did you mean by 'crash'? An exception or a segfault or equivalent?

    I believe the finalization code was reworked a bit last fall, so could you determine whether or not there is still a problem in 3.2.1? Do not bother with 3.1.4 unless you think this is a security problem.

    @grahamd
    Copy link
    Mannequin Author

    grahamd mannequin commented Jun 27, 2011

    Segmentation fault. The original description explains the problem is dereferencing of a NULL pointer which has a tendency to invoke such behaviour.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 16, 2012

    There seem to be two issues at play here:

    • the atexit module (and its companion helper _Py_PyAtExit()) doesn't know about sub-interpreters.

    • PyState_FindModule() doesn't know about sub-interpreters either, because the m_index field (which records the module's index in an interpreter's module list (PyInterpreterState.modules_by_index)) is recorded in the PyModuleDef structure rather than the module instance: it is therefore global to all interpreters

    Having atexit work properly with sub-interpreters would require fixing these two issues AFAICT.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 16, 2012

    Hmm, it seems I may be mistaken about the second point. m_index is actually generated such that all modules end up in the same position in the interpreters' respective modules_by_index lists. Sorry.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 17, 2012

    Hmm something else: currently the atexit funcs are only called when the main interpreter exits, but that doesn't really make sense: if I register a function from a sub-interpreter, why would it execute correctly from another interpreter? All kind of fun things can happen...

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 17, 2012

    New changeset eb47af6e9e22 by Antoine Pitrou in branch '3.2':
    Test running of code in a sub-interpreter
    http://hg.python.org/cpython/rev/eb47af6e9e22

    New changeset a108818aaa0d by Antoine Pitrou in branch 'default':
    Test running of code in a sub-interpreter
    http://hg.python.org/cpython/rev/a108818aaa0d

    @pitrou
    Copy link
    Member

    pitrou commented Jan 17, 2012

    Here is a patch for subinterp-wise atexit functions.
    (I haven't got any specific test for the crash but the patch adds a test and it works)

    @grahamd
    Copy link
    Mannequin Author

    grahamd mannequin commented Jan 18, 2012

    What are the intentions with respect to atexit and sub interpreters?

    The original report was only about ensuring that the main interpreter doesn't crash if an atexit function was registered in a sub interpreter. So, was not expecting a change to sub interpreters in submitting this report, in as much as atexit callbacks for sub interpreters are never invoked in Python 2.X.

    That said, for mod_wsgi I have extended sub interpreter destruction so that atexit callbacks registered in sub interpreters are called. For mod_wsgi though, sub interpreters are only destroyed on process shutdown. For the general case, a sub interpreter could be destroyed at any time during the life of the process. If one called atexit callbacks on such sub interpreter destruction, it notionally changes the meaning of atexit, which is in process exit and not really sub interpreter exit.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 18, 2012

    That said, for mod_wsgi I have extended sub interpreter destruction so
    that atexit callbacks registered in sub interpreters are called. For
    mod_wsgi though, sub interpreters are only destroyed on process
    shutdown. For the general case, a sub interpreter could be destroyed
    at any time during the life of the process. If one called atexit
    callbacks on such sub interpreter destruction, it notionally changes
    the meaning of atexit, which is in process exit and not really sub
    interpreter exit.

    Well the atexit docs say "Functions thus registered are automatically
    executed upon normal interpreter termination".

    My reasoning is:

    • atexit functions are already called at interpreter destruction (*),
      not at process shutdown. If you call Py_Initialize and Py_Finalize
      several times in a row, you will have several atexit calls.

    • registering a function in an interpreter and calling it in another is
      undefined and potentially dangerous. That function can for example refer
      to global objects which are different from the calling interpreter's
      global objects. These objects or their internal structures could have
      become invalid when the sub-interpreter was shut down.

    I expect uses of sub-interpreters to be quite rare apart from mod_wsgi,
    so following mod_wsgi makes sense IMO.

    () note atexit functions are even called *before module globals are
    garbage collected. It's quite early in the cleanup phase.

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Dec 6, 2017

    See also https://bugs.python.org/issue31901, which reached a similar conclusion to this discussion (i.e. atexit functions should be run when the subinterpreter goes away).

    @Dormouse759
    Copy link
    Mannequin

    Dormouse759 mannequin commented Dec 11, 2017

    At a guess, the problem is because in atexit_callfuncs():

    module = PyState_FindModule(&atexitmodule);
    if (module == NULL)
    return;

    In bpo-31901, I solved this problem by getiing rid of PyState_FindModule in the atexit code, using module state and PEP-489 multiphase initialization.

    This needed some changes in the pystate and pylifecycle code, so the specific interpreter has a reference to its own atexit module. This way, Py_EndInterpreter() is able to call the callbacks for the given interpreter.

    See: #4611

    @encukou encukou closed this as completed Sep 10, 2018
    @vstinner vstinner added topic-subinterpreters and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels May 15, 2020
    @vstinner vstinner changed the title atexit_callfuncs() crashing within Py_Finalize() when using multiple interpreters. [subinterpreters] atexit_callfuncs() crashing within Py_Finalize() when using multiple interpreters. May 15, 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
    Labels
    topic-subinterpreters type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants