mirrors/cpython
1
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-08-31 05:58:33 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

gh-121794: Don't set ob_tid to zero in fast-path dealloc (#121799)

Browse source 
We should maintain the invariant that a zero `ob_tid` implies the
refcount fields are merged.

* Move the assignment in `_Py_MergeZeroLocalRefcount` to immediately
  before the refcount merge.
* Update `_PyTrash_thread_destroy_chain` to set `ob_ref_shared` to
  `_Py_REF_MERGED` when setting `ob_tid` to zero.

Also check this invariant with assertions in the GC in debug builds.
That uncovered a bug when running out of memory during GC.
This commit is contained in:
Sam Gross 2024-07-15 17:50:10 -04:00 • committed by GitHub
parent 82a4dac9f6
commit d23be3947c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 62 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

8
Objects/object.c
View file

@ -375,7 +375,6 @@ _Py_MergeZeroLocalRefcount(PyObject *op)
{
assert(op->ob_ref_local == 0);
_Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);
Py_ssize_t shared = _Py_atomic_load_ssize_acquire(&op->ob_ref_shared);
if (shared == 0) {
// Fast-path: shared refcount is zero (including flags)
@ -383,6 +382,11 @@ _Py_MergeZeroLocalRefcount(PyObject *op)
return;
}
// gh-121794: This must be before the store to `ob_ref_shared` (gh-119999),
// but should outside the fast-path to maintain the invariant that
// a zero `ob_tid` implies a merged refcount.
_Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);
// Slow-path: atomically set the flags (low two bits) to _Py_REF_MERGED.
Py_ssize_t new_shared;
do {
@ -2724,7 +2728,6 @@ _PyTrash_thread_deposit_object(PyThreadState *tstate, PyObject *op)
_PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op));
_PyObject_ASSERT(op, Py_REFCNT(op) == 0);
#ifdef Py_GIL_DISABLED
_PyObject_ASSERT(op, op->ob_tid == 0);
op->ob_tid = (uintptr_t)tstate->delete_later;
#else
_PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)tstate->delete_later);
@ -2757,6 +2760,7 @@ _PyTrash_thread_destroy_chain(PyThreadState *tstate)
#ifdef Py_GIL_DISABLED
tstate->delete_later = (PyObject*) op->ob_tid;
op->ob_tid = 0;
_Py_atomic_store_ssize_relaxed(&op->ob_ref_shared, _Py_REF_MERGED);
#else
tstate->delete_later = (PyObject*) _PyGCHead_PREV(_Py_AS_GC(op));
#endif