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] Can Py_Finalize() be called if the current interpreter is not the main interpreter? #83046

Closed
vstinner opened this issue Nov 20, 2019 · 10 comments
Labels
3.9 only security fixes topic-subinterpreters

Comments

@vstinner
Copy link
Member

BPO 38865
Nosy @ncoghlan, @vstinner, @ericsnowcurrently, @nanjekyejoannah, @shihai1991, @LewisGaul
PRs
  • [WIP] bpo-38865: Py_FinalizeEx() cannot be called in a subinterpreter #19063
  • gh-80406: Finalise subinterpreters in Py_FinalizeEx() #17575
  • 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 = None
    created_at = <Date 2019-11-20.15:42:03.952>
    labels = ['expert-subinterpreters', '3.9']
    title = '[subinterpreters] Can Py_Finalize() be called if the current interpreter is not the main interpreter?'
    updated_at = <Date 2022-01-21.12:35:14.327>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-01-21.12:35:14.327>
    actor = 'vstinner'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Subinterpreters']
    creation = <Date 2019-11-20.15:42:03.952>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38865
    keywords = ['patch']
    message_count = 9.0
    messages = ['357080', '357331', '357332', '357333', '364573', '364588', '364676', '365037', '411121']
    nosy_count = 6.0
    nosy_names = ['ncoghlan', 'vstinner', 'eric.snow', 'nanjekyejoannah', 'shihai1991', 'LewisGaul']
    pr_nums = ['19063', '17575']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue38865'
    versions = ['Python 3.9']

    @vstinner
    Copy link
    Member Author

    Programs/_testembed.c contains the following test used by test_embed:

    static int test_audit_subinterpreter(void)
    {
        Py_IgnoreEnvironmentFlag = 0;
        PySys_AddAuditHook(_audit_subinterpreter_hook, NULL);
        _testembed_Py_Initialize();
    
        Py_NewInterpreter(); 
        Py_NewInterpreter(); 
        Py_NewInterpreter(); 
     
        Py_Finalize(); 
    
        switch (_audit_subinterpreter_interpreter_count) {
            case 3: return 0;
            case 0: return -1;
            default: return _audit_subinterpreter_interpreter_count;
        }
    }

    When Py_Finalize() is called, the current interpreter is a subinterpreter (the 3rd interpreter), not the main interpreter.

    • Is it correct to call Py_Finalize() in such case?
    • Is Python supposed to magically destroy the 3 interpreters?

    In bpo-38858, I'm trying to reuse the same code to initialize and finalize the "main" interpreter and subinterpreters. I had an issue with test_audit_subinterpreter() when working on the PR 17293.

    I modified my PR 17293 to not expect that Py_Finalize() can only be called from the main interpreter, but actually check if the current interpreter is the main interpreter or not. It fix test_audit_subinterpreter() but again, I'm not sure what is the correct behavior?

    --

    Last year, we had a similar discussion about calling Py_Main() *after* Py_Initialize(). I hacked the code to make it possible because it was supported previously, even if the Py_Main() configuration is only partially applied. But I understood that Nick Coghlan would prefer to deprecate supporting to call Py_Initialize() before Py_Main().

    PEP-587 added Py_RunMain() which provides a different solution to this problem:
    https://docs.python.org/dev/c-api/init_config.html#c.Py_RunMain

    @vstinner vstinner added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.9 only security fixes labels Nov 20, 2019
    @ericsnowcurrently
    Copy link
    Member

    tl;dr Py_Finalize() probably *should* only be allowed under the main interpreter.

    As you know well, when the runtime starts we do it at first relative to a partially initialized main interpreter and the finish initialization with a fully operational main interpreter. My expectation is that some of the global runtime state relies somehow on the main interpreter's state. Consequently, finalization would then need to happen under the main interpreter.

    The same idea applies to fork-without-exec, where we destroy all interpreters but the current one. We now only allow os.fork() in the main interpreter. So I'd expect the same treatement for finalization.

    On the other hand, if the "main" interpreter isn't special after runtime init is done (i.e. a subinterpreter is effectively indistinguishable by nature) then it certainly would not matter which one finalizes (or is active during fork() for that matter). On top of that, the effort we are making to identify the main interpreter (both in the code and in the docs) becomes unnecessary.

    If we look at it from that angle, though, we do have cases where the main interpreter is still significant (regardless of the relationship between the main interpreter and the global runtime state). At the very least, the main interpreter is solely responsible for some global resources, like signals. So the main interpreter cannot go away until nothing else in the runtime relies on those resources. I expect that we won't go down that route, which means finalization should happen only under the main interpreter.

    Also keep in mind that this only matters relative to the C-API and I expect only embedders (not extensions) will be finalizing the runtime. With PEP-554 the only case where you would run into problems is when running subinterpreters in daemon threads. And we already know about problems with daemon threads during runtime finalization! :)

    @ericsnowcurrently
    Copy link
    Member

    • Is Python supposed to magically destroy the 3 interpreters?

    Doesn't it? Gah. I guess I was thinking of PyOS_AfterFork_Child(), which calls _PyInterpreterState_DeleteExceptMain(). :/ In September there was a nice comment right above Py_FinalizeEx() saying that we do not clean up other interpreters or threads, but I haven't checked when it got removed.

    @ericsnowcurrently
    Copy link
    Member

    I could swear the topic of destroy-everything-in-PyFinalize has come up before but I don't remember anything specific. Doing so there sure makes sense though...

    @vstinner
    Copy link
    Member Author

    This issue became a blocker issue while working on bpo-39984, when I tried to move pending calls from _PyRuntimeState to PyInterpreterState.

    I wrote PR 19063 to deny calling Py_FinalizeEx() from a subinterpreter.

    Eric:

    On the other hand, if the "main" interpreter isn't special after runtime init is done (i.e. a subinterpreter is effectively indistinguishable by nature) then it certainly would not matter which one finalizes (or is active during fork() for that matter). On top of that, the effort we are making to identify the main interpreter (both in the code and in the docs) becomes unnecessary.

    Well. Maybe we will be able to reach this point later, but right now, there are concrete implementation issues. The main interpreter remains special. Only the main interpreter is allowed to call fork(), spawn daemon threads, handle signals, etc.

    If we manage to make subinterpreters less special, we can change Py_FinalizeEx() again later.

    I asked:

    Is Python supposed to magically destroy the 3 interpreters?

    Currently, Py_FinalizeEx() fails with a fatal error if there are remaining subinterpreters:
    ---
    Fatal Python error: PyInterpreterState_Delete: remaining subinterpreters
    Python runtime state: finalizing (tstate=0xfaa1c0)

    Abandon (core dumped)
    ---

    IMHO it's a good behavior.

    if we want, we can modify Py_FinalizeEx() to magically end subinterpters later. The opposite may be more annoying for embedders.

    @vstinner
    Copy link
    Member Author

    See also bpo-36225: "Lingering subinterpreters should be implicitly cleared on shutdown".

    @vstinner
    Copy link
    Member Author

    I changed my mind. I managed to implement bpo-39984 without needing my PR 19063, so I closed it.

    @vstinner
    Copy link
    Member Author

    See also bpo-37776: Test Py_Finalize() from a subinterpreter.

    @vstinner vstinner added topic-subinterpreters and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels May 15, 2020
    @vstinner vstinner changed the title Can Py_Finalize() be called if the current interpreter is not the main interpreter? [subinterpreters] Can Py_Finalize() be called if the current interpreter is not the main interpreter? May 15, 2020
    @vstinner
    Copy link
    Member Author

    I marked bpo-37776 "[subinterpreters] Test Py_Finalize() from a subinterpreter" as a duplicate of this issue.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 3, 2022

    I don't have the bandwidth to work on this issue, I just close it.

    @vstinner vstinner closed this as completed Nov 3, 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 topic-subinterpreters
    Projects
    Status: Done
    Development

    No branches or pull requests

    2 participants