diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 71bd01884426ad..b9f494c4059d95 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -5,6 +5,7 @@ PyAPI_FUNC(void) _Py_NewReference(PyObject *op); PyAPI_FUNC(void) _Py_NewReferenceNoTotal(PyObject *op); PyAPI_FUNC(void) _Py_ResurrectReference(PyObject *op); +PyAPI_FUNC(void) _Py_ForgetReference(PyObject *op); #ifdef Py_REF_DEBUG /* These are useful as debugging aids when chasing down refleaks. */ diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index ffd31bd4a27f49..53403ebcfc0043 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -730,6 +730,9 @@ _PyObject_ResurrectStart(PyObject *op) #else Py_SET_REFCNT(op, 1); #endif +#ifdef Py_TRACE_REFS + _Py_ResurrectReference(op); +#endif } // Undoes an object resurrection by decrementing the refcount without calling @@ -743,13 +746,22 @@ _PyObject_ResurrectEnd(PyObject *op) #endif #ifndef Py_GIL_DISABLED Py_SET_REFCNT(op, Py_REFCNT(op) - 1); - return Py_REFCNT(op) != 0; + if (Py_REFCNT(op) == 0) { +# ifdef Py_TRACE_REFS + _Py_ForgetReference(op); +# endif + return 0; + } + return 1; #else uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); Py_ssize_t shared = _Py_atomic_load_ssize_acquire(&op->ob_ref_shared); if (_Py_IsOwnedByCurrentThread(op) && local == 1 && shared == 0) { // Fast-path: object has a single refcount and is owned by this thread _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0); +# ifdef Py_TRACE_REFS + _Py_ForgetReference(op); +# endif return 0; } // Slow-path: object has a shared refcount or is not owned by this thread diff --git a/Objects/object.c b/Objects/object.c index 16aedac916bf34..d4a5bc19baf105 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -496,10 +496,22 @@ _PyObject_ResurrectEndSlow(PyObject *op) // merge the refcount. This isn't necessary in all cases, but it // simplifies the implementation. Py_ssize_t refcount = _Py_ExplicitMergeRefcount(op, -1); - return refcount != 0; + if (refcount == 0) { +#ifdef Py_TRACE_REFS + _Py_ForgetReference(op); +#endif + return 0; + } + return 1; } int is_dead = _Py_DecRefSharedIsDead(op, NULL, 0); - return !is_dead; + if (is_dead) { +#ifdef Py_TRACE_REFS + _Py_ForgetReference(op); +#endif + return 0; + } + return 1; } @@ -589,20 +601,24 @@ PyObject_CallFinalizerFromDealloc(PyObject *self) Py_REFCNT(self) > 0, "refcount is too small"); + _PyObject_ASSERT(self, + (!_PyType_IS_GC(Py_TYPE(self)) + || _PyObject_GC_IS_TRACKED(self))); + /* Undo the temporary resurrection; can't use DECREF here, it would * cause a recursive call. */ - if (!_PyObject_ResurrectEnd(self)) { - return 0; /* this is the normal path out */ + if (_PyObject_ResurrectEnd(self)) { + /* tp_finalize resurrected it! + gh-130202: Note that the object may still be dead in the free + threaded build in some circumstances, so it's not safe to access + `self` after this point. For example, the last reference to the + resurrected `self` may be held by another thread, which can + concurrently deallocate it. */ + return -1; } - /* tp_finalize resurrected it! Make it look like the original Py_DECREF - * never happened. */ - _Py_ResurrectReference(self); - - _PyObject_ASSERT(self, - (!_PyType_IS_GC(Py_TYPE(self)) - || _PyObject_GC_IS_TRACKED(self))); - return -1; + /* this is the normal path out, the caller continues with deallocation. */ + return 0; } int @@ -2616,11 +2632,10 @@ _Py_ResurrectReference(PyObject *op) #endif } - -#ifdef Py_TRACE_REFS void _Py_ForgetReference(PyObject *op) { +#ifdef Py_TRACE_REFS if (Py_REFCNT(op) < 0) { _PyObject_ASSERT_FAILED_MSG(op, "negative refcnt"); } @@ -2636,8 +2651,11 @@ _Py_ForgetReference(PyObject *op) #endif _PyRefchain_Remove(interp, op); +#endif } + +#ifdef Py_TRACE_REFS static int _Py_PrintReference(_Py_hashtable_t *ht, const void *key, const void *value,