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

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.

diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h
index d59ab490493ba488efa6cb8f3240ad33a01c85a8..30db3f2b6701ecbd1b9e20a1163705489a5d501f 100644 (file)
--- a/Include/internal/pycore_pymem.h
+++ b/Include/internal/pycore_pymem.h
@@ -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

diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index ee4fe49a57f2af5ded97c0f5ec7bdd74118349fc..1528cf6e20ee609cc69e77500dc3aa5e38b3fae4 100644 (file)
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -1361,6 +1361,34 @@ class GCTogglingTests(unittest.TestCase):
             # 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()
@@ -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

diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-05-21-01-42-45.bpo-44184.9qOptC.rst b/Misc/NEWS.d/next/Core and Builtins/2021-05-21-01-42-45.bpo-44184.9qOptC.rst
new file mode 100644 (file)
index 0000000..3aba9a5
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2021-05-21-01-42-45.bpo-44184.9qOptC.rst
@@ -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.

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 84be0a1a2f52381b285efe756976ff5e6f4fdf6d..af99ab79d14ade88b6b9e5320d59541c2e3d030d 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -1446,6 +1446,12 @@ 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);
 
@@ -1453,8 +1459,9 @@ subtype_dealloc(PyObject *self)
        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