bpo-41796: Call _PyAST_Fini() earlier to fix a leak

Call _PyAST_Fini() on all interpreters, not only on the main interpreter. Also, call it ealier to fix a reference leak. Python types contain a reference to themselves in in their PyTypeObject.tp_mro member. _PyAST_Fini() must called before the last GC collection to destroy AST types. _PyInterpreterState_Clear() now calls _PyAST_Fini(). It now also calls _PyWarnings_Fini() on subinterpeters, not only on the main interpreter. Add an assertion in AST init_types() to ensure that the _ast module is no longer used after _PyAST_Fini() has been called.
python · vstinner · Nov 3, 2020 · Nov 3, 2020 · Nov 3, 2020 · ef4b7b304da9070c3969f332390457ca658a2f1f
commit ef4b7b304da9070c3969f332390457ca658a2f1f
diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h
@@ -84,7 +84,7 @@ extern void _PyFaulthandler_Fini(void);
 extern void _PyHash_Fini(void);
 extern void _PyTraceMalloc_Fini(void);
 extern void _PyWarnings_Fini(PyInterpreterState *interp);
-extern void _PyAST_Fini(PyThreadState *tstate);
+extern void _PyAST_Fini(PyInterpreterState *interp);
 
 extern PyStatus _PyGILState_Init(PyThreadState *tstate);
 extern void _PyGILState_Fini(PyThreadState *tstate);

diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py
@@ -1015,18 +1015,35 @@ def visitModule(self, mod):
 
 """, 0, reflow=False)
 
-        self.emit("static int init_types(struct ast_state *state)",0)
-        self.emit("{", 0)
-        self.emit("if (state->initialized) return 1;", 1)
-        self.emit("if (init_identifiers(state) < 0) return 0;", 1)
-        self.emit("state->AST_type = PyType_FromSpec(&AST_type_spec);", 1)
-        self.emit("if (!state->AST_type) return 0;", 1)
-        self.emit("if (add_ast_fields(state) < 0) return 0;", 1)
+        self.file.write(textwrap.dedent('''
+            static int
+            init_types(struct ast_state *state)
+            {
+                // init_types() must not be called after _PyAST_Fini()
+                // has been called
+                assert(state->initialized >= 0);
+
+                if (state->initialized) {
+                    return 1;
+                }
+                if (init_identifiers(state) < 0) {
+                    return 0;
+                }
+                state->AST_type = PyType_FromSpec(&AST_type_spec);
+                if (!state->AST_type) {
+                    return 0;
+                }
+                if (add_ast_fields(state) < 0) {
+                    return 0;
+                }
+        '''))
         for dfn in mod.dfns:
             self.visit(dfn)
-        self.emit("state->initialized = 1;", 1)
-        self.emit("return 1;", 1);
-        self.emit("}", 0)
+        self.file.write(textwrap.dedent('''
+                state->initialized = 1;
+                return 1;
+            }
+        '''))
 
     def visitProduct(self, prod, name):
         if prod.fields:
@@ -1353,23 +1370,27 @@ def generate_ast_state(module_state, f):
 
 
 def generate_ast_fini(module_state, f):
-    f.write("""
-void _PyAST_Fini(PyThreadState *tstate)
-{
-#ifdef Py_BUILD_CORE
-    struct ast_state *state = &tstate->interp->ast;
-#else
-    struct ast_state *state = &global_ast_state;
-#endif
-
-""")
+    f.write(textwrap.dedent("""
+            void _PyAST_Fini(PyInterpreterState *interp)
+            {
+            #ifdef Py_BUILD_CORE
+                struct ast_state *state = &interp->ast;
+            #else
+                struct ast_state *state = &global_ast_state;
+            #endif
+
+    """))
     for s in module_state:
         f.write("    Py_CLEAR(state->" + s + ');\n')
-    f.write("""
-    state->initialized = 0;
-}
+    f.write(textwrap.dedent("""
+            #if defined(Py_BUILD_CORE) && !defined(NDEBUG)
+                state->initialized = -1;
+            #else
+                state->initialized = 0;
+            #endif
+            }
 
-""")
+    """))
 
 
 def generate_module_def(mod, f, internal_h):

diff --git a/Python/Python-ast.c b/Python/Python-ast.c
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
@@ -1545,12 +1545,6 @@ flush_std_files(void)
 static void
 finalize_interp_types(PyThreadState *tstate)
 {
-    // The _ast module state is shared by all interpreters.
-    // The state must only be cleared by the main interpreter.
-    if (_Py_IsMainInterpreter(tstate)) {
-        _PyAST_Fini(tstate);
-    }
-
     _PyExc_Fini(tstate);
     _PyFrame_Fini(tstate);
     _PyAsyncGen_Fini(tstate);
@@ -1591,8 +1585,6 @@ finalize_interp_clear(PyThreadState *tstate)
         _Py_ClearFileSystemEncoding();
     }
 
-    _PyWarnings_Fini(tstate->interp);
-
     finalize_interp_types(tstate);
 }
 

diff --git a/Python/pystate.c b/Python/pystate.c
@@ -300,13 +300,16 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
     Py_CLEAR(interp->after_forkers_parent);
     Py_CLEAR(interp->after_forkers_child);
 #endif
-    if (_PyRuntimeState_GetFinalizing(runtime) == NULL) {
-        _PyWarnings_Fini(interp);
-    }
+
+    _PyAST_Fini(interp);
+    _PyWarnings_Fini(interp);
+
+    // All Python types must be destroyed before the last GC collection. Python
+    // types create a reference cycle to themselves in their in their
+    // PyTypeObject.tp_mro member (the tuple contains the type).
 
     /* 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.