diff options
| author | Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> | 2024-07-15 22:15:23 (GMT) |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-07-15 22:15:23 (GMT) |
| commit | 0794220a6935a25bd9667be60b0c67403bc73f47 (patch) | |
| tree | abfbf472fe853d9c1ac9ecec8ec831c4d54ae1c7 /Objects/object.c | |
| parent | 6396c77571aee99386a7b5a5863bbe0b8c5df4b1 (diff) | |
| download | cpython-0794220a6935a25bd9667be60b0c67403bc73f47.zip cpython-0794220a6935a25bd9667be60b0c67403bc73f47.tar.gz cpython-0794220a6935a25bd9667be60b0c67403bc73f47.tar.bz2 | |
[3.13] gh-121794: Don't set `ob_tid` to zero in fast-path dealloc (GH-121799) (#121821)
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 d23be3947ced081914f4458c84f729c9c37f0219)
Co-authored-by: Sam Gross <colesbury@gmail.com>
Diffstat (limited to 'Objects/object.c')
| -rw-r--r-- | Objects/object.c | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/Objects/object.c b/Objects/object.c index fcd81b8..2c40da5 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -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 |