[Python-checkins] bpo-41796: Call _PyAST_Fini() earlier to fix a leak (GH-23131)
vstinner
webhook-mailer at python.org
Tue Nov 3 12:07:20 EST 2020
https://github.com/python/cpython/commit/fd957c124c44441d9c5eaf61f7af8cf266bafcb1
commit: fd957c124c44441d9c5eaf61f7af8cf266bafcb1
branch: master
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2020-11-03T18:07:15+01:00
summary:
bpo-41796: Call _PyAST_Fini() earlier to fix a leak (GH-23131)
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.
files:
M Include/internal/pycore_pylifecycle.h
M Parser/asdl_c.py
M Python/Python-ast.c
M Python/pylifecycle.c
M Python/pystate.c
diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h
index 6d84e37232b30..cba3bbdc2b256 100644
--- 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
index 9a833e841de41..9fec7ae017cd7 100755
--- 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
index f04addbe2011d..a456b51951425 100644
--- a/Python/Python-ast.c
+++ b/Python/Python-ast.c
@@ -261,10 +261,10 @@ get_ast_state(void)
#include "Python-ast.h"
#include "structmember.h"
-void _PyAST_Fini(PyThreadState *tstate)
+void _PyAST_Fini(PyInterpreterState *interp)
{
#ifdef Py_BUILD_CORE
- struct ast_state *state = &tstate->interp->ast;
+ struct ast_state *state = &interp->ast;
#else
struct ast_state *state = &global_ast_state;
#endif
@@ -483,7 +483,11 @@ void _PyAST_Fini(PyThreadState *tstate)
Py_CLEAR(state->vararg);
Py_CLEAR(state->withitem_type);
+#if defined(Py_BUILD_CORE) && !defined(NDEBUG)
+ state->initialized = -1;
+#else
state->initialized = 0;
+#endif
}
static int init_identifiers(struct ast_state *state)
@@ -1227,13 +1231,27 @@ static int add_ast_fields(struct ast_state *state)
}
-static int init_types(struct ast_state *state)
+
+static int
+init_types(struct ast_state *state)
{
- if (state->initialized) return 1;
- if (init_identifiers(state) < 0) return 0;
+ // 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;
+ if (!state->AST_type) {
+ return 0;
+ }
+ if (add_ast_fields(state) < 0) {
+ return 0;
+ }
state->mod_type = make_type(state, "mod", state->AST_type, NULL, 0,
"mod = Module(stmt* body, type_ignore* type_ignores)\n"
" | Interactive(stmt* body)\n"
@@ -1902,6 +1920,7 @@ static int init_types(struct ast_state *state)
TypeIgnore_fields, 2,
"TypeIgnore(int lineno, string tag)");
if (!state->TypeIgnore_type) return 0;
+
state->initialized = 1;
return 1;
}
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index ff58c1b9153bd..cad0fa7026bfd 100644
--- 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
index e37cbd5a65787..c9882a7f74bc2 100644
--- 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.
More information about the Python-checkins
mailing list