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;