From 5bcd788fe8092c0125db0043b56fa9ef9442206a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 21 May 2021 01:37:42 +0200 Subject: [PATCH 1/4] bpo-44184: Fix subtype_dealloc() for freed type 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. --- .../2021-05-21-01-42-45.bpo-44184.9qOptC.rst | 3 +++ Objects/typeobject.c | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-05-21-01-42-45.bpo-44184.9qOptC.rst 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 index 00000000000000..3aba9a58475c61 --- /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 84be0a1a2f5238..af99ab79d14ade 100644 --- 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 From 454e996d221aaa73bdaae217e0a59684fe77bdee Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 21 May 2021 18:11:30 +0200 Subject: [PATCH 2/4] Add an unit test --- Lib/test/test_gc.py | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index ee4fe49a57f2af..fd25c651b4bc04 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1361,6 +1361,36 @@ 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 builtins + import sys + import os + + # 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. + # Fork callbacks are only cleared at the end of + # interpreter_clear(). + def callback(): + pass + callback.a = x + os.register_at_fork(after_in_child=callback) + """) + assert_python_ok("-c", code) + + def test_main(): enabled = gc.isenabled() gc.disable() @@ -1370,7 +1400,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 From b670383abfc58028ab0056748e2ba8401600dc6e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 21 May 2021 18:15:27 +0200 Subject: [PATCH 3/4] _PyMem_IsPtrFreed() argument is now const --- Include/internal/pycore_pymem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index d59ab490493ba4..30db3f2b6701ec 100644 --- 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 From efd45ad7881c4c1b364637f970b33cc5aeeb301d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 21 May 2021 18:34:25 +0200 Subject: [PATCH 4/4] Port the unit test to Windows --- Lib/test/test_gc.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index fd25c651b4bc04..1528cf6e20ee60 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1371,9 +1371,7 @@ def test_ast_fini(self): # _PyAST_Fini() which clears references to AST types. code = textwrap.dedent(""" import ast - import builtins - import sys - import os + import codecs # Small AST tree to keep their AST types alive tree = ast.parse("def f(x, y): return 2*x-y") @@ -1381,12 +1379,12 @@ def test_ast_fini(self): x.append(x) # Put the cycle somewhere to survive until the last GC collection. - # Fork callbacks are only cleared at the end of + # Codec search functions are only cleared at the end of # interpreter_clear(). - def callback(): - pass - callback.a = x - os.register_at_fork(after_in_child=callback) + def search_func(encoding): + return None + search_func.a = x + codecs.register(search_func) """) assert_python_ok("-c", code)