Skip to content

Commit

Permalink
bpo-41796: Call _PyAST_Fini() earlier to fix a leak (GH-23131)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vstinner authored Nov 3, 2020
1 parent 212d32f commit fd957c1
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 44 deletions.
2 changes: 1 addition & 1 deletion Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
69 changes: 45 additions & 24 deletions Parser/asdl_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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):
Expand Down
33 changes: 26 additions & 7 deletions Python/Python-ast.c

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -1591,8 +1585,6 @@ finalize_interp_clear(PyThreadState *tstate)
_Py_ClearFileSystemEncoding();
}

_PyWarnings_Fini(tstate->interp);

finalize_interp_types(tstate);
}

Expand Down
11 changes: 7 additions & 4 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit fd957c1

Please sign in to comment.