[Python-checkins] bpo-42208: Call GC collect earlier in PyInterpreterState_Clear() (GH-23044)

Fri Oct 30 17:51:06 EDT 2020

https://github.com/python/cpython/commit/eba5bf2f5672bf4861c626937597b85ac0c242b9
commit: eba5bf2f5672bf4861c626937597b85ac0c242b9
branch: master
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2020年10月30日T22:51:02+01:00
summary:
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().
files:
M Include/internal/pycore_interp.h
M Modules/gcmodule.c
M Python/pylifecycle.c
M Python/pystate.c

diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index eee369a44bfc7..69d2108da4322 100644
--- a/Include/internal/pycore_interp.h
+++ b/Include/internal/pycore_interp.h
@@ -267,6 +267,7 @@ extern PyStatus _PyInterpreterState_SetConfig(
 PyInterpreterState *interp,
 const PyConfig *config);
 
+extern void _PyInterpreterState_Clear(PyThreadState *tstate);
 
 
 /* cross-interpreter data registry */
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index d90ff33684fe8..e6ad0f2dd4222 100644
--- a/Modules/gcmodule.c
+++ b/Modules/gcmodule.c
@@ -1191,6 +1191,11 @@ gc_collect_main(PyThreadState *tstate, int generation,
 _PyTime_t t1 = 0; /* initialize to prevent a compiler warning */
 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
 if (tstate->interp->config._isolated_interpreter) {
 // bpo-40533: The garbage collector must not be run on parallel on
@@ -2073,16 +2078,13 @@ PyGC_Collect(void)
 Py_ssize_t
 _PyGC_CollectNoFail(PyThreadState *tstate)
 {
- assert(!_PyErr_Occurred(tstate));
-
- GCState *gcstate = &tstate->interp->gc;
-
 /* Ideally, this function is only called on interpreter shutdown,
 and therefore not recursively. Unfortunately, when there are daemon
 threads, a daemon thread can start a cyclic garbage collection
 during interpreter shutdown (and then never finish it).
 See http://bugs.python.org/issue8713#msg195178 for an example.
 */
+ GCState *gcstate = &tstate->interp->gc;
 if (gcstate->collecting) {
 return 0;
 }
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index adef1617f6132..ff58c1b9153bd 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1576,10 +1576,7 @@ finalize_interp_clear(PyThreadState *tstate)
 int is_main_interp = _Py_IsMainInterpreter(tstate);
 
 /* Clear interpreter state and all thread states */
- PyInterpreterState_Clear(tstate->interp);
-
- /* Last explicit GC collection */
- _PyGC_CollectNoFail(tstate);
+ _PyInterpreterState_Clear(tstate);
 
 /* Clear all loghooks */
 /* Both _PySys_Audit function and users still need PyObject, such as tuple.
@@ -1588,8 +1585,6 @@ finalize_interp_clear(PyThreadState *tstate)
 _PySys_ClearAuditHooks(tstate);
 }
 
- _PyGC_Fini(tstate);
-
 if (is_main_interp) {
 _Py_HashRandomization_Fini();
 _PyArg_Fini();
diff --git a/Python/pystate.c b/Python/pystate.c
index eb24f2b800607..e88898670cdff 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -268,14 +268,11 @@ PyInterpreterState_New(void)
 }
 
 
-void
-PyInterpreterState_Clear(PyInterpreterState *interp)
+static void
+interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
 {
 _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) {
 _PyErr_Clear(tstate);
 }
@@ -306,6 +303,12 @@ PyInterpreterState_Clear(PyInterpreterState *interp)
 if (_PyRuntimeState_GetFinalizing(runtime) == NULL) {
 _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.
 Because clearing other attributes can execute arbitrary Python code
 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
 zapthreads(PyInterpreterState *interp, int check_current)
 {
