classification
Title: atexit callbacks should be run at subinterpreter shutdown
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Dormouse759, eric.snow, grahamd, ncoghlan, petr.viktorin, pitrou, xgdomingo
Priority: normal Keywords: patch

Created on 2017-10-30 15:44 by Dormouse759, last changed 2017-12-20 10:18 by pitrou. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4611 merged Dormouse759, 2017-11-28 14:50
Messages (11)
msg305233 - (view) Author: Marcel Plch (Dormouse759) * Date: 2017-10-30 15:44
`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?
msg305277 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-10-31 08:12
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)
msg305278 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-10-31 08:14
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.
msg305279 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2017-10-31 09:28
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.)
msg305280 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-10-31 09:46
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).
msg305298 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2017-10-31 14:06
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.
msg305859 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2017-11-08 15:49
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.
msg305896 - (view) Author: Graham Dumpleton (grahamd) Date: 2017-11-08 20:07
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
msg305927 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-11-09 00:35
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.
msg307574 - (view) Author: Marcel Plch (Dormouse759) * Date: 2017-12-04 15:52
I created a PR with fix on this issue - https://github.com/python/cpython/pull/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().
msg308714 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-12-20 10:18
New changeset 776407fe893fd42972c7e3f71423d9d86741d07c by Antoine Pitrou (Marcel Plch) in branch 'master':
bpo-31901: atexit callbacks should be run at subinterpreter shutdown (#4611)
https://github.com/python/cpython/commit/776407fe893fd42972c7e3f71423d9d86741d07c
History
Date User Action Args
2017-12-20 10:18:52pitrousetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-12-20 10:18:05pitrousetnosy: + pitrou
messages: + msg308714
2017-12-13 03:54:23xgdomingosetnosy: + xgdomingo
2017-12-04 15:52:34Dormouse759setmessages: + msg307574
2017-11-28 14:50:38Dormouse759setkeywords: + patch
stage: patch review
pull_requests: + pull_request4527
2017-11-09 00:35:26ncoghlansetmessages: + msg305927
components: + Interpreter Core, - Extension Modules
title: atexit callbacks only called for current subinterpreter -> atexit callbacks should be run at subinterpreter shutdown
2017-11-08 20:07:09grahamdsetmessages: + msg305896
2017-11-08 15:49:56petr.viktorinsetmessages: + msg305859
2017-10-31 14:06:07petr.viktorinsetmessages: + msg305298
2017-10-31 09:46:06ncoghlansetmessages: + msg305280
2017-10-31 09:28:40petr.viktorinsetmessages: + msg305279
2017-10-31 08:14:02ncoghlansetmessages: + msg305278
2017-10-31 08:12:37ncoghlansetnosy: + grahamd, eric.snow
messages: + msg305277
2017-10-30 15:44:43Dormouse759create