https://github.com/python/cpython/commit/d4c72fed8cba8e15ab7bb6c30a92bc9f2c8f0a2c
commit: d4c72fed8cba8e15ab7bb6c30a92bc9f2c8f0a2c
branch: main
author: Peter Bierma <[email protected]>
committer: vstinner <[email protected]>
date: 2024-11-15T11:21:30+01:00
summary:

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 <[email protected]>

files:
A 
Misc/NEWS.d/next/Core_and_Builtins/2024-11-02-14-43-46.gh-issue-126312.LMHzLT.rst
M Lib/test/test_gc.py
M Python/gc_free_threading.c

diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index 2b3c0d3baddeaf..0372815b9bfd27 100644
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -1082,6 +1082,44 @@ def __del__(self):
         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 00000000000000..19c8f0a348731c
--- /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 986d80c18d36c8..499ee51fdb2cd4 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++;
     }

_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]

Reply via email to