From ecda3ae2a51cdec65996005690391172bc25ca4b Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Fri, 15 Nov 2024 08:46:00 -0500 Subject: [PATCH] [3.13] gh-126312: Don't traverse frozen objects on the free-threaded build (GH-126338) (#126866) * Fix merge conflicts. * [3.13] gh-126312: Don't traverse frozen objects on the free-threaded build (GH-126338) Also, _PyGC_Freeze() no longer freezes unreachable objects. (cherry picked from commit d4c72fed8cba8e15ab7bb6c30a92bc9f2c8f0a2c) Co-authored-by: Peter Bierma Co-authored-by: Sergey B Kirpichev --------- Co-authored-by: Sergey B Kirpichev --- Lib/test/test_gc.py | 39 +++++++++++++++++++ ...-11-02-14-43-46.gh-issue-126312.LMHzLT.rst | 2 + Python/gc_free_threading.c | 17 ++++++-- 3 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-11-02-14-43-46.gh-issue-126312.LMHzLT.rst diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 9ce9d938f8b..3e7dda3bac2 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1129,6 +1129,45 @@ class GCTests(unittest.TestCase): gc.collect() self.assertTrue(collected) + def test_traverse_frozen_objects(self): + # See GH-126312: Objects that were not frozen could traverse over + # a frozen object on the free-threaded build, which would cause + # a negative reference count. + x = [1, 2, 3] + gc.freeze() + y = [x] + y.append(y) + del y + gc.collect() + gc.unfreeze() + + def test_deferred_refcount_frozen(self): + # Also from GH-126312: objects that use deferred reference counting + # weren't ignored if they were frozen. Unfortunately, it's pretty + # difficult to come up with a case that triggers this. + # + # Calling gc.collect() while the garbage collector is frozen doesn't + # trigger this normally, but it *does* if it's inside unittest for whatever + # reason. We can't call unittest from inside a test, so it has to be + # in a subprocess. + source = textwrap.dedent(""" + import gc + import unittest + + + class Test(unittest.TestCase): + def test_something(self): + gc.freeze() + gc.collect() + gc.unfreeze() + + + if __name__ == "__main__": + unittest.main() + """) + assert_python_ok("-c", source) + + class GCCallbackTests(unittest.TestCase): def setUp(self): # Save gc state and disable it. diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-02-14-43-46.gh-issue-126312.LMHzLT.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-02-14-43-46.gh-issue-126312.LMHzLT.rst new file mode 100644 index 00000000000..19c8f0a3487 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-02-14-43-46.gh-issue-126312.LMHzLT.rst @@ -0,0 +1,2 @@ +Fix crash during garbage collection on an object frozen by :func:`gc.freeze` on the +free-threaded build. diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index bf3cd01f0d0..ff86c759eb2 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -110,6 +110,12 @@ worklist_remove(struct worklist_iter *iter) iter->next = iter->ptr; } +static inline int +gc_is_frozen(PyObject *op) +{ + return (op->ob_gc_bits & _PyGC_BITS_FROZEN) != 0; +} + static inline int gc_is_unreachable(PyObject *op) { @@ -229,7 +235,7 @@ op_from_block(void *block, void *arg, bool include_frozen) if (!_PyObject_GC_IS_TRACKED(op)) { return NULL; } - if (!include_frozen && (op->ob_gc_bits & _PyGC_BITS_FROZEN) != 0) { + if (!include_frozen && gc_is_frozen(op)) { return NULL; } return op; @@ -364,7 +370,10 @@ process_delayed_frees(PyInterpreterState *interp) static int visit_decref(PyObject *op, void *arg) { - if (_PyObject_GC_IS_TRACKED(op) && !_Py_IsImmortal(op)) { + if (_PyObject_GC_IS_TRACKED(op) + && !_Py_IsImmortal(op) + && !gc_is_frozen(op)) + { // If update_refs hasn't reached this object yet, mark it // as (tentatively) unreachable and initialize ob_tid to zero. gc_maybe_init_refs(op); @@ -1419,7 +1428,7 @@ visit_freeze(const mi_heap_t *heap, const mi_heap_area_t *area, void *block, size_t block_size, void *args) { PyObject *op = op_from_block(block, args, true); - if (op != NULL) { + if (op != NULL && !gc_is_unreachable(op)) { op->ob_gc_bits |= _PyGC_BITS_FROZEN; } return true; @@ -1464,7 +1473,7 @@ visit_count_frozen(const mi_heap_t *heap, const mi_heap_area_t *area, void *block, size_t block_size, void *args) { PyObject *op = op_from_block(block, args, true); - if (op != NULL && (op->ob_gc_bits & _PyGC_BITS_FROZEN) != 0) { + if (op != NULL && gc_is_frozen(op)) { struct count_frozen_args *arg = (struct count_frozen_args *)args; arg->count++; }