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

[3.13] gh-121794: Don't set ob_tid to zero in fast-path dealloc (GH-121799) (#121821)

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.
(cherry picked from commit d23be3947c)

Co-authored-by: Sam Gross <colesbury@gmail.com>
This commit is contained in:
Miss Islington (bot) 2024-07-16 00:15:23 +02:00 • committed by GitHub
parent 6396c77571
commit 0794220a69
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 {
@ -2739,7 +2743,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);
@ -2772,6 +2775,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