From d4c72fed8cba8e15ab7bb6c30a92bc9f2c8f0a2c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Fri, 15 Nov 2024 05:21:30 -0500 Subject: [PATCH] gh-126312: Don't traverse frozen objects on the free-threaded build (#126338) Also, _PyGC_Freeze() no longer freezes unreachable objects. Co-authored-by: Sergey B Kirpichev --- Lib/test/test_gc.py | 38 +++++++++++++++++++ ...-11-02-14-43-46.gh-issue-126312.LMHzLT.rst | 2 + Python/gc_free_threading.c | 19 +++++++--- 3 files changed, 54 insertions(+), 5 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 2b3c0d3badd..0372815b9bf 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1082,6 +1082,44 @@ 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 IncrementalGCTests(unittest.TestCase): 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 986d80c18d3..499ee51fdb2 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -113,6 +113,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) { @@ -277,7 +283,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; @@ -358,7 +364,7 @@ gc_visit_stackref(_PyStackRef stackref) // being dead already. if (PyStackRef_IsDeferred(stackref) && !PyStackRef_IsNull(stackref)) { PyObject *obj = PyStackRef_AsPyObjectBorrow(stackref); - if (_PyObject_GC_IS_TRACKED(obj)) { + if (_PyObject_GC_IS_TRACKED(obj) && !gc_is_frozen(obj)) { gc_add_refs(obj, 1); } } @@ -439,7 +445,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); @@ -1539,7 +1548,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; @@ -1584,7 +1593,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++; }