gh-130202: Fix bug in `_PyObject_ResurrectEnd` in free threaded build… · python/cpython@f963239 · GitHub
Skip to content

Commit f963239

Browse files
authored
gh-130202: Fix bug in _PyObject_ResurrectEnd in free threaded build (gh-130281)
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.
1 parent c5f925c commit f963239

3 files changed

Lines changed: 46 additions & 15 deletions

File tree

Include/cpython/object.h

Lines changed: 1 addition & 0 deletions

Include/internal/pycore_object.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,9 @@ _PyObject_ResurrectStart(PyObject *op)
730730
#else
731731
Py_SET_REFCNT(op, 1);
732732
#endif
733+
#ifdef Py_TRACE_REFS
734+
_Py_ResurrectReference(op);
735+
#endif
733736
}
734737

735738
// Undoes an object resurrection by decrementing the refcount without calling
@@ -743,13 +746,22 @@ _PyObject_ResurrectEnd(PyObject *op)
743746
#endif
744747
#ifndef Py_GIL_DISABLED
745748
Py_SET_REFCNT(op, Py_REFCNT(op) - 1);
746-
return Py_REFCNT(op) != 0;
749+
if (Py_REFCNT(op) == 0) {
750+
# ifdef Py_TRACE_REFS
751+
_Py_ForgetReference(op);
752+
# endif
753+
return 0;
754+
}
755+
return 1;
747756
#else
748757
uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local);
749758
Py_ssize_t shared = _Py_atomic_load_ssize_acquire(&op->ob_ref_shared);
750759
if (_Py_IsOwnedByCurrentThread(op) && local == 1 && shared == 0) {
751760
// Fast-path: object has a single refcount and is owned by this thread
752761
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
762+
# ifdef Py_TRACE_REFS
763+
_Py_ForgetReference(op);
764+
# endif
753765
return 0;
754766
}
755767
// Slow-path: object has a shared refcount or is not owned by this thread

Objects/object.c

Lines changed: 32 additions & 14 deletions

0 commit comments

Comments
 (0)