bpo-31901: atexit callbacks should be run at subinterpreter shutdown #4611
Conversation
Include/pylifecycle.h
Outdated
| @@ -81,7 +81,7 @@ PyAPI_FUNC(void) Py_EndInterpreter(PyThreadState *); | |||
| * exit functions. | |||
| */ | |||
| #ifndef Py_LIMITED_API | |||
| PyAPI_FUNC(void) _Py_PyAtExit(void (*func)(void)); | |||
| PyAPI_FUNC(void) _Py_PyAtExit(PyObject *, void (*func)(PyObject *)); | |||
There was a problem hiding this comment.
A nit, but it would seem to me more conventional to take the user data (the PyObject *) as second argument.
Programs/_testembed.c
Outdated
| @@ -196,6 +195,88 @@ static int test_bpo20891(void) | |||
| } | |||
|
|
|||
|
|
|||
| static int test_atexit_callbacks(void) { | |||
There was a problem hiding this comment.
Is it actually necessary to use C code for this? As opposed to calling support.run_in_subinterp from test_atexit (as already done in two other tests).
There was a problem hiding this comment.
Awesome, didn't know about this thing, it is rewritten in Python now.
|
Thank you for the PR! I posted a couple comments. |
|
There was a crash on AppVeyor. I've retried the bulld in case it's an unrelated sporadic issue (?). |
|
Looks like the AppVeyor crash isn't sporadic after all (and your PR is the only one to trigger it). Do you have a Windows machine or VM to test? |
|
By the way, analyzing this PR led me to #4826 :-) |
|
Unfortunately, I don't have any Windows machine. |
|
PR #4826 was merged, so you just have to rebase/merge this PR on git master (you will have to resolve conflicts, which should be relatively easy here). |
|
And about the Windows crash? |
|
If it persists after git master is merged in, I'll try to reproduce on my Windows VM (if I manage to unstuck it). |
|
Ok, I think I've found the culprit for the Windows crash: now that you're using PEP 489 multiphase init support, |
|
Note I don't know exactly why that happens, so I asked at https://mail.python.org/pipermail/python-dev/2017-December/151238.html |
|
@pitrou this PR is ready for review |
|
This looks good to me. I'm gonna merge soon if no other core developer chimes in. |
|
The documentation issue is tracked in bpo-32374. |
This PR moves the atexit callbacks from Python runtime context to interpreter state.
Also, Py_EndInterpreter() now calls atexit callbacks of the interpreter.
These changes make it possible to implement PEP-489 multiphase initialization for atexit module, which is also done in this PR.
https://bugs.python.org/issue31901