https://github.com/python/cpython/commit/f554237b8ef6c60df651ac17eb0ef0c095cef185
commit: f554237b8ef6c60df651ac17eb0ef0c095cef185
branch: main
author: Mark Shannon <m...@hotpy.org>
committer: markshannon <m...@hotpy.org>
date: 2025-05-05T13:44:50+01:00
summary:

GH-133261: Make sure that the GC doesn't untrack objects in trashcan (GH-133431)

files:
A 
Misc/NEWS.d/next/Core_and_Builtins/2025-05-05-12-03-46.gh-issue-133261.bL1gqz.rst
M Objects/object.c
M Python/gc.c

diff --git 
a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-05-12-03-46.gh-issue-133261.bL1gqz.rst
 
b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-05-12-03-46.gh-issue-133261.bL1gqz.rst
new file mode 100644
index 00000000000000..1f40ea7eb8843b
--- /dev/null
+++ 
b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-05-12-03-46.gh-issue-133261.bL1gqz.rst
@@ -0,0 +1,3 @@
+Fix bug where the cycle GC could untrack objects in the trashcan because
+they looked like they were immortal. When objects are added to the trashcan,
+we take care to ensure they keep a mortal reference count.
diff --git a/Objects/object.c b/Objects/object.c
index 0974a231ec101a..f5fe25eb5aaf8f 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -2930,6 +2930,33 @@ Py_ReprLeave(PyObject *obj)
 
 /* Trashcan support. */
 
+#ifndef Py_GIL_DISABLED
+/* We need to store a pointer in the refcount field of
+ * an object. It is important that we never store 0 (NULL).
+* It is also important to not make the object appear immortal,
+* or it might be untracked by the cycle GC. */
+static uintptr_t
+pointer_to_safe_refcount(void *ptr)
+{
+    uintptr_t full = (uintptr_t)ptr;
+    assert((full & 3) == 0);
+    uint32_t refcnt = (uint32_t)full;
+    if (refcnt >= (uint32_t)_Py_IMMORTAL_MINIMUM_REFCNT) {
+        full = full - ((uintptr_t)_Py_IMMORTAL_MINIMUM_REFCNT) + 1;
+    }
+    return full + 2;
+}
+
+static void *
+safe_refcount_to_pointer(uintptr_t refcnt)
+{
+    if (refcnt & 1) {
+        refcnt += _Py_IMMORTAL_MINIMUM_REFCNT - 1;
+    }
+    return (void *)(refcnt - 2);
+}
+#endif
+
 /* Add op to the gcstate->trash_delete_later list.  Called when the current
  * call-stack depth gets large.  op must be a currently untracked gc'ed
  * object, with refcount 0.  Py_DECREF must already have been called on it.
@@ -2941,11 +2968,10 @@ _PyTrash_thread_deposit_object(PyThreadState *tstate, 
PyObject *op)
 #ifdef Py_GIL_DISABLED
     op->ob_tid = (uintptr_t)tstate->delete_later;
 #else
-    /* Store the delete_later pointer in the refcnt field.
-     * As this object may still be tracked by the GC,
-     * it is important that we never store 0 (NULL). */
-    uintptr_t refcnt = (uintptr_t)tstate->delete_later;
-    *((uintptr_t*)op) = refcnt+1;
+    /* Store the delete_later pointer in the refcnt field. */
+    uintptr_t refcnt = pointer_to_safe_refcount(tstate->delete_later);
+    *((uintptr_t*)op) = refcnt;
+    assert(!_Py_IsImmortal(op));
 #endif
     tstate->delete_later = op;
 }
@@ -2967,7 +2993,7 @@ _PyTrash_thread_destroy_chain(PyThreadState *tstate)
         /* Get the delete_later pointer from the refcnt field.
          * See _PyTrash_thread_deposit_object(). */
         uintptr_t refcnt = *((uintptr_t*)op);
-        tstate->delete_later = (PyObject *)(refcnt - 1);
+        tstate->delete_later = safe_refcount_to_pointer(refcnt);
         op->ob_refcnt = 0;
 #endif
 
diff --git a/Python/gc.c b/Python/gc.c
index b7b48c8af39c4a..7b0e6d6e803504 100644
--- a/Python/gc.c
+++ b/Python/gc.c
@@ -493,6 +493,7 @@ update_refs(PyGC_Head *containers)
         next = GC_NEXT(gc);
         PyObject *op = FROM_GC(gc);
         if (_Py_IsImmortal(op)) {
+            assert(!_Py_IsStaticImmortal(op));
             _PyObject_GC_UNTRACK(op);
            gc = next;
            continue;

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com

Reply via email to