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.
(cherry picked from commit 615069eb08494d089bf24e43547fbc482ed699b8)

Co-authored-by: Victor Stinner <vstinner@python.org>

diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h
index 3d925e2250d25218bef240b535d93ac2d71ea388..9bcb5f5efd742964d31f3d477ded029a60c757cd 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 38c9cb7123311b618a85921a72a12c61a22cc078..59cff20e6a7e7ffa4bb996790dc7977f994ac6f6 100644 (file)
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -1360,6 +1360,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()
@@ -1369,7 +1397,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 82faa7987ff50693e2f08d779e31befd6a305784..f201515d7db330138c911739168e8709bfff30ed 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -1341,6 +1341,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);
 
@@ -1348,8 +1354,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