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

atexit callbacks should be run at subinterpreter shutdown #76082

Closed
Dormouse759 mannequin opened this issue Oct 30, 2017 · 11 comments
Closed

atexit callbacks should be run at subinterpreter shutdown #76082

Dormouse759 mannequin opened this issue Oct 30, 2017 · 11 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@Dormouse759
Copy link
Mannequin

Dormouse759 mannequin commented Oct 30, 2017

BPO 31901
Nosy @ncoghlan, @pitrou, @encukou, @ericsnowcurrently, @Dormouse759, @xgid
PRs
  • bpo-31901: atexit callbacks should be run at subinterpreter shutdown #4611
  • 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-12-20.10:18:52.338>
    created_at = <Date 2017-10-30.15:44:43.120>
    labels = ['interpreter-core', 'type-bug', '3.7']
    title = 'atexit callbacks should be run at subinterpreter shutdown'
    updated_at = <Date 2017-12-20.10:18:52.337>
    user = 'https://github.com/Dormouse759'

    bugs.python.org fields:

    activity = <Date 2017-12-20.10:18:52.337>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-12-20.10:18:52.338>
    closer = 'pitrou'
    components = ['Interpreter Core']
    creation = <Date 2017-10-30.15:44:43.120>
    creator = 'Dormouse759'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31901
    keywords = ['patch']
    message_count = 11.0
    messages = ['305233', '305277', '305278', '305279', '305280', '305298', '305859', '305896', '305927', '307574', '308714']
    nosy_count = 7.0
    nosy_names = ['ncoghlan', 'pitrou', 'grahamd', 'petr.viktorin', 'eric.snow', 'Dormouse759', 'xgdomingo']
    pr_nums = ['4611']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue31901'
    versions = ['Python 3.7']

    @Dormouse759
    Copy link
    Mannequin Author

    Dormouse759 mannequin commented Oct 30, 2017

    Py_FinalizeEx() only executes callbacks from the subinterpreter from which Py_FinalizeEx is called.

    What is the expected behavior here?
    Should the callbacks be specific to individual subinterpreters?

    @Dormouse759 Dormouse759 mannequin added extension-modules C modules in the Modules dir 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Oct 30, 2017
    @ncoghlan
    Copy link
    Contributor

    Hmm, I don't think calling Py_Finalize in any interpreter other than the main one is actually defined at all, so I'm inclined to just make it an error, rather than trying to figure out how it should work. It would then be up to the embedding application to make sure it switched back to the main interpreter before calling Py_Finalize.

    (We don't have this concern with Py_Initialize, since that *creates* the main interpreter)

    @ncoghlan
    Copy link
    Contributor

    For the main interpreter, Py_Finalize should be destroying all other subinterpreters as one of the first things it does, so if we're *not* currently doing that, it would make sense to fix it as part of Eric's subinterpreters PEP.

    @encukou
    Copy link
    Member

    encukou commented Oct 31, 2017

    I don't have a reason to think Py_Finalize is not *destroying* the other subinterpreters. But it's not calling their atexit callbacks.

    (Is there any reason Py_Finalize couldn't switch to the main interpreter itself? Assuming it's even necessary.)

    @ncoghlan
    Copy link
    Contributor

    I guess we allow an unhandled SystemExit in a child thread to propagate to (and hence terminate) the main thread, so allowing a Py_Finalize call in a subinterpreter to terminate the main interpreter would be comparable to that.

    My main rationale for *requiring* that the main interpreter be active (or be made active) when shutting down is to reduce the number of scenarios we need to test (right now we only test Py_Initialize/Py_Finalize cycles with a single interpreter, and officially allowing finalization from arbitrary interpreters expands that test matrix a fair bit).

    @encukou
    Copy link
    Member

    encukou commented Oct 31, 2017

    I'm not sure where the concept of "main subinterpreter" comes in, with respect to this issue.

    I thnik the issue of atexit callbacks could be solved by something like keeping info about each callback's subinterpreter, and switching subinterpreters before running each one. I can see the locking needed for that being quite hairy, but independent of which thread calls all this.

    @encukou
    Copy link
    Member

    encukou commented Nov 8, 2017

    When you destroy a subinterpreter before Py_Finalize is called, Python can't start calling its atexit callbacks – they no longer have a subinterpreter to run in.

    Therefore I think callbacks for a particular subinterpreter should be called when (and only when) that subinterpreter is destroyed. Regardless of whether it's the main one or not.

    @grahamd
    Copy link
    Mannequin

    grahamd mannequin commented Nov 8, 2017

    FWIW, that atexit callbacks were not called for sub interpreters ever was a pain point for mod_wsgi.

    What mod_wsgi does is override the destruction sequence so that it will first go through each sub interpreter when and shutdown threading explicitly, then call atexit handlers. When that is done, only then will it destroy the sub interpreter and the main interpreter.

    I have noted this previously in discussion associated with:

    https://bugs.python.org/issue6531

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Nov 9, 2017

    Ah, I see - yeah, that makes a lot of sense.

    I've retitled the issue to make it clearer that the problem is where the responsibility for calling atexit functions lives - currently it's in Py_Finalize, but really it should be inside the interpreter object's own teardown code.

    @ncoghlan ncoghlan added interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed extension-modules C modules in the Modules dir labels Nov 9, 2017
    @ncoghlan ncoghlan changed the title atexit callbacks only called for current subinterpreter atexit callbacks should be run at subinterpreter shutdown Nov 9, 2017
    @Dormouse759
    Copy link
    Mannequin Author

    Dormouse759 mannequin commented Dec 4, 2017

    I created a PR with fix on this issue - #4611

    This makes Py_EndInterpreter() call atexit callbacks for the subinterpreter it is destroying.

    It doesn't make Py_Finalize() end all subinterpreters, as the current implementation of subinterpreters makes it hard to do so. This is the same as the current behaviour: you need to end all subinterpreters before calling Py_Finalize().

    @pitrou
    Copy link
    Member

    pitrou commented Dec 20, 2017

    New changeset 776407f by Antoine Pitrou (Marcel Plch) in branch 'master':
    bpo-31901: atexit callbacks should be run at subinterpreter shutdown (bpo-4611)
    776407f

    @pitrou pitrou closed this as completed Dec 20, 2017
    @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
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants