Skip to content

bpo-44184: Fix subtype_dealloc() for freed type #26274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Include/internal/pycore_pymem.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ PyAPI_FUNC(int) _PyMem_SetDefaultAllocator(
fills newly allocated memory with CLEANBYTE (0xCD) and newly freed memory
with DEADBYTE (0xDD). Detect also "untouchable bytes" marked
with FORBIDDENBYTE (0xFD). */
static inline int _PyMem_IsPtrFreed(void *ptr)
static inline int _PyMem_IsPtrFreed(const void *ptr)
{
uintptr_t value = (uintptr_t)ptr;
#if SIZEOF_VOID_P == 8
Expand Down
34 changes: 33 additions & 1 deletion Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,34 @@ def __del__(self):
# empty __dict__.
self.assertEqual(x, None)


class PythonFinalizationTests(unittest.TestCase):
def test_ast_fini(self):
# bpo-44184: Regression test for subtype_dealloc() when deallocating
# an AST instance also destroy its AST type: subtype_dealloc() must
# not access the type memory after deallocating the instance, since
# the type memory can be freed as well. The test is also related to
# _PyAST_Fini() which clears references to AST types.
code = textwrap.dedent("""
import ast
import codecs

# Small AST tree to keep their AST types alive
tree = ast.parse("def f(x, y): return 2*x-y")
x = [tree]
x.append(x)

# Put the cycle somewhere to survive until the last GC collection.
# Codec search functions are only cleared at the end of
# interpreter_clear().
def search_func(encoding):
return None
search_func.a = x
codecs.register(search_func)
""")
assert_python_ok("-c", code)


def test_main():
enabled = gc.isenabled()
gc.disable()
Expand All @@ -1370,7 +1398,11 @@ def test_main():

try:
gc.collect() # Delete 2nd generation garbage
run_unittest(GCTests, GCTogglingTests, GCCallbackTests)
run_unittest(
GCTests,
GCCallbackTests,
GCTogglingTests,
PythonFinalizationTests)
finally:
gc.set_debug(debug)
# test gc.enable() even if GC is disabled by default
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a crash at Python exit when a deallocator function removes the last strong
reference to a heap type.
Patch by Victor Stinner.
11 changes: 9 additions & 2 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1446,15 +1446,22 @@ subtype_dealloc(PyObject *self)
if (_PyType_IS_GC(base)) {
_PyObject_GC_TRACK(self);
}

// Don't read type memory after calling basedealloc() since basedealloc()
// can deallocate the type and free its memory.
int type_needs_decref = (type->tp_flags & Py_TPFLAGS_HEAPTYPE
&& !(base->tp_flags & Py_TPFLAGS_HEAPTYPE));

assert(basedealloc);
basedealloc(self);

/* Can't reference self beyond this point. It's possible tp_del switched
our type from a HEAPTYPE to a non-HEAPTYPE, so be careful about
reference counting. Only decref if the base type is not already a heap
allocated type. Otherwise, basedealloc should have decref'd it already */
if (type->tp_flags & Py_TPFLAGS_HEAPTYPE && !(base->tp_flags & Py_TPFLAGS_HEAPTYPE))
Py_DECREF(type);
if (type_needs_decref) {
Py_DECREF(type);
}

endlabel:
Py_TRASHCAN_END
Expand Down