bpo-42208: Call GC collect earlier in PyInterpreterState_Clear() (GH-23044)

The last GC collection is now done before clearing builtins and sys
dictionaries. Add also assertions to ensure that gc.collect() is no
longer called after _PyGC_Fini().

Pass also the tstate to PyInterpreterState_Clear() to pass the
correct tstate to _PyGC_CollectNoFail() and _PyGC_Fini().
This commit is contained in:
Victor Stinner 2020-10-30 22:51:02 +01:00 committed by GitHub
parent 4fe72090de
commit eba5bf2f56
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 15 deletions

View File

@ -267,6 +267,7 @@ extern PyStatus _PyInterpreterState_SetConfig(
PyInterpreterState *interp, PyInterpreterState *interp,
const PyConfig *config); const PyConfig *config);
extern void _PyInterpreterState_Clear(PyThreadState *tstate);
/* cross-interpreter data registry */ /* cross-interpreter data registry */

View File

@ -1191,6 +1191,11 @@ gc_collect_main(PyThreadState *tstate, int generation,
_PyTime_t t1 = 0; /* initialize to prevent a compiler warning */ _PyTime_t t1 = 0; /* initialize to prevent a compiler warning */
GCState *gcstate = &tstate->interp->gc; GCState *gcstate = &tstate->interp->gc;
// gc_collect_main() must not be called before _PyGC_Init
// or after _PyGC_Fini()
assert(gcstate->garbage != NULL);
assert(!_PyErr_Occurred(tstate));
#ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS #ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS
if (tstate->interp->config._isolated_interpreter) { if (tstate->interp->config._isolated_interpreter) {
// bpo-40533: The garbage collector must not be run on parallel on // bpo-40533: The garbage collector must not be run on parallel on
@ -2073,16 +2078,13 @@ PyGC_Collect(void)
Py_ssize_t Py_ssize_t
_PyGC_CollectNoFail(PyThreadState *tstate) _PyGC_CollectNoFail(PyThreadState *tstate)
{ {
assert(!_PyErr_Occurred(tstate));
GCState *gcstate = &tstate->interp->gc;
/* Ideally, this function is only called on interpreter shutdown, /* Ideally, this function is only called on interpreter shutdown,
and therefore not recursively. Unfortunately, when there are daemon and therefore not recursively. Unfortunately, when there are daemon
threads, a daemon thread can start a cyclic garbage collection threads, a daemon thread can start a cyclic garbage collection
during interpreter shutdown (and then never finish it). during interpreter shutdown (and then never finish it).
See http://bugs.python.org/issue8713#msg195178 for an example. See http://bugs.python.org/issue8713#msg195178 for an example.
*/ */
GCState *gcstate = &tstate->interp->gc;
if (gcstate->collecting) { if (gcstate->collecting) {
return 0; return 0;
} }

View File

@ -1576,10 +1576,7 @@ finalize_interp_clear(PyThreadState *tstate)
int is_main_interp = _Py_IsMainInterpreter(tstate); int is_main_interp = _Py_IsMainInterpreter(tstate);
/* Clear interpreter state and all thread states */ /* Clear interpreter state and all thread states */
PyInterpreterState_Clear(tstate->interp); _PyInterpreterState_Clear(tstate);
/* Last explicit GC collection */
_PyGC_CollectNoFail(tstate);
/* Clear all loghooks */ /* Clear all loghooks */
/* Both _PySys_Audit function and users still need PyObject, such as tuple. /* Both _PySys_Audit function and users still need PyObject, such as tuple.
@ -1588,8 +1585,6 @@ finalize_interp_clear(PyThreadState *tstate)
_PySys_ClearAuditHooks(tstate); _PySys_ClearAuditHooks(tstate);
} }
_PyGC_Fini(tstate);
if (is_main_interp) { if (is_main_interp) {
_Py_HashRandomization_Fini(); _Py_HashRandomization_Fini();
_PyArg_Fini(); _PyArg_Fini();

View File

@ -268,14 +268,11 @@ out_of_memory:
} }
void static void
PyInterpreterState_Clear(PyInterpreterState *interp) interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
{ {
_PyRuntimeState *runtime = interp->runtime; _PyRuntimeState *runtime = interp->runtime;
/* Use the current Python thread state to call audit hooks,
not the current Python thread state of 'interp'. */
PyThreadState *tstate = _PyThreadState_GET();
if (_PySys_Audit(tstate, "cpython.PyInterpreterState_Clear", NULL) < 0) { if (_PySys_Audit(tstate, "cpython.PyInterpreterState_Clear", NULL) < 0) {
_PyErr_Clear(tstate); _PyErr_Clear(tstate);
} }
@ -306,6 +303,12 @@ PyInterpreterState_Clear(PyInterpreterState *interp)
if (_PyRuntimeState_GetFinalizing(runtime) == NULL) { if (_PyRuntimeState_GetFinalizing(runtime) == NULL) {
_PyWarnings_Fini(interp); _PyWarnings_Fini(interp);
} }
/* Last garbage collection on this interpreter */
_PyGC_CollectNoFail(tstate);
_PyGC_Fini(tstate);
/* We don't clear sysdict and builtins until the end of this function. /* We don't clear sysdict and builtins until the end of this function.
Because clearing other attributes can execute arbitrary Python code Because clearing other attributes can execute arbitrary Python code
which requires sysdict and builtins. */ which requires sysdict and builtins. */
@ -320,6 +323,25 @@ PyInterpreterState_Clear(PyInterpreterState *interp)
} }
void
PyInterpreterState_Clear(PyInterpreterState *interp)
{
// Use the current Python thread state to call audit hooks and to collect
// garbage. It can be different than the current Python thread state
// of 'interp'.
PyThreadState *current_tstate = _PyThreadState_GET();
interpreter_clear(interp, current_tstate);
}
void
_PyInterpreterState_Clear(PyThreadState *tstate)
{
interpreter_clear(tstate->interp, tstate);
}
static void static void
zapthreads(PyInterpreterState *interp, int check_current) zapthreads(PyInterpreterState *interp, int check_current)
{ {