Skip to content

Commit

Permalink
bpo-44184: Fix subtype_dealloc() for freed type (GH-26274)
Browse files Browse the repository at this point in the history
Fix a crash at Python exit when a deallocator function removes the
last strong reference to a heap type.

Don't read type memory after calling basedealloc() since
basedealloc() can deallocate the type and free its memory.

_PyMem_IsPtrFreed() argument is now constant.
  • Loading branch information
vstinner committed May 21, 2021
1 parent 642fdfd commit 615069e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Include/internal/pycore_pymem.h
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
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
@@ -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
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

0 comments on commit 615069e

Please sign in to comment.