classification
Title: atexit_callfuncs() crashing within Py_Finalize() when using multiple interpreters.
Type: crash Stage: patch review
Components: Interpreter Core Versions: Python 3.6, Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Dormouse759, chn, encukou, eric.snow, grahamd, loewis, ncoghlan, pitrou, python-dev, r.david.murray, terry.reedy
Priority: normal Keywords: patch

Created on 2009-07-21 11:19 by grahamd, last changed 2017-12-11 16:09 by Dormouse759.

Files
File name Uploaded Description Edit
atexitsubinterps.patch pitrou, 2012-01-17 23:29
Messages (15)
msg90753 - (view) Author: Graham Dumpleton (grahamd) Date: 2009-07-21 11:19
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.
msg90757 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-07-21 12:29
I strongly recommend you move to 3.1.  3.0 will not be getting any more
bug patches.
msg90758 - (view) Author: Graham Dumpleton (grahamd) Date: 2009-07-21 12:44
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.
msg90759 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-07-21 13:24
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 ;)
msg139225 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-26 21:14
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.
msg139231 - (view) Author: Graham Dumpleton (grahamd) Date: 2011-06-27 00:07
Segmentation fault. The original description explains the problem is dereferencing of a NULL pointer which has a tendency to invoke such behaviour.
msg151395 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-16 17:52
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.
msg151396 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-16 18:00
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.
msg151494 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-17 23:12
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...
msg151495 - (view) Author: Roundup Robot (python-dev) Date: 2012-01-17 23:24
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
msg151496 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-17 23:29
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)
msg151527 - (view) Author: Graham Dumpleton (grahamd) Date: 2012-01-18 09:49
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.
msg151531 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-18 10:56
> 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.
msg307702 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-12-06 06:30
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).
msg308057 - (view) Author: Marcel Plch (Dormouse759) * Date: 2017-12-11 16:09
> At a guess, the problem is because in atexit_callfuncs():
> 
>    module = PyState_FindModule(&atexitmodule);
>    if (module == NULL)
>        return;

In #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: https://github.com/python/cpython/pull/4611
History
Date User Action Args
2017-12-11 16:09:05Dormouse759setmessages: + msg308057
2017-12-07 15:44:12encukousetnosy: + encukou, Dormouse759
2017-12-06 06:30:22ncoghlansetnosy: + ncoghlan
messages: + msg307702
2015-07-03 00:32:22eric.snowsetversions: + Python 3.5, Python 3.6, - Python 3.2, Python 3.3
2012-01-18 10:56:48pitrousetmessages: + msg151531
2012-01-18 09:49:14grahamdsetmessages: + msg151527
2012-01-17 23:29:20pitrousetfiles: + atexitsubinterps.patch
keywords: + patch
messages: + msg151496

stage: patch review
2012-01-17 23:24:09python-devsetnosy: + python-dev
messages: + msg151495
2012-01-17 23:12:40pitrousetmessages: + msg151494
2012-01-17 00:14:58eric.snowsetnosy: + eric.snow
2012-01-16 18:00:04pitrousetmessages: + msg151396
2012-01-16 17:52:32pitrousetnosy: + pitrou, loewis

messages: + msg151395
versions: + Python 3.2, Python 3.3
2012-01-16 14:10:18chnsetnosy: + chn
2011-06-27 00:07:51grahamdsetmessages: + msg139231
2011-06-26 21:14:39terry.reedysetnosy: + terry.reedy

messages: + msg139225
versions: - Python 3.0, Python 3.1
2009-07-21 13:24:18r.david.murraysetmessages: + msg90759
2009-07-21 12:44:19grahamdsetmessages: + msg90758
2009-07-21 12:29:48r.david.murraysetnosy: + r.david.murray
messages: + msg90757
2009-07-21 11:19:28grahamdcreate