Skip to content

gh-130202: Fix bug in _PyObject_ResurrectEnd in free threaded build #130281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Include/cpython/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
14 changes: 13 additions & 1 deletion Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
46 changes: 32 additions & 14 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
Expand All @@ -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,
Expand Down
Loading