mirrors/cpython
1
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-07-07 19:35:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 8

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

Browse source 
This fixes a fairly subtle bug involving finalizers and resurrection in
debug free threaded builds: if `_PyObject_ResurrectEnd` returns `1`
(i.e., the object was resurrected by a finalizer), it's not safe to
access the object because it might still be deallocated. For example:

 * The finalizer may have exposed the object to another thread. That
   thread may hold the last reference and concurrently deallocate it any
   time after `_PyObject_ResurrectEnd()` returns `1`.
 * `_PyObject_ResurrectEnd()` may call `_Py_brc_queue_object()`, which
   may internally deallocate the object immediately if the owning thread
   is dead.

Therefore, it's important not to access the object after it's
resurrected. We only violate this in two cases, and only in debug
builds:

 * We assert that the object is tracked appropriately. This is now moved
   up betewen the finalizer and the `_PyObject_ResurrectEnd()` call.

 * The `--with-trace-refs` builds may need to remember the object if
   it's resurrected. This is now handled by `_PyObject_ResurrectStart()`
   and `_PyObject_ResurrectEnd()`.

Note that `--with-trace-refs` is currently disabled in `--disable-gil`
builds because the refchain hash table isn't thread-safe, but this
refactoring avoids an additional thread-safety issue.
This commit is contained in:
Sam Gross 2025-02-25 12:03:28 -05:00 • committed by GitHub
parent c5f925c8c9
commit f963239ff1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 46 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

46
Objects/object.c
View file

@ -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
@ -2601,11 +2617,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");
}
@ -2621,8 +2636,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,