Skip to content

Commit 4055577

Browse files
authored
gh-119999: Fix potential race condition in _Py_ExplicitMergeRefcount (#120000)
We need to write to `ob_ref_local` and `ob_tid` before `ob_ref_shared`. Once we mark `ob_ref_shared` as merged, some other thread may free the object because the caller also passes in `-1` as `extra` to give up its only reference.
1 parent 109e108 commit 4055577

File tree

1 file changed

+11
-8
lines changed

1 file changed

+11
-8
lines changed

Objects/object.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -401,24 +401,27 @@ Py_ssize_t
401401
_Py_ExplicitMergeRefcount(PyObject *op, Py_ssize_t extra)
402402
{
403403
assert(!_Py_IsImmortal(op));
404+
405+
#ifdef Py_REF_DEBUG
406+
_Py_AddRefTotal(_PyThreadState_GET(), extra);
407+
#endif
408+
409+
// gh-119999: Write to ob_ref_local and ob_tid before merging the refcount.
410+
Py_ssize_t local = (Py_ssize_t)op->ob_ref_local;
411+
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
412+
_Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);
413+
404414
Py_ssize_t refcnt;
405415
Py_ssize_t new_shared;
406416
Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared);
407417
do {
408418
refcnt = Py_ARITHMETIC_RIGHT_SHIFT(Py_ssize_t, shared, _Py_REF_SHARED_SHIFT);
409-
refcnt += (Py_ssize_t)op->ob_ref_local;
419+
refcnt += local;
410420
refcnt += extra;
411421

412422
new_shared = _Py_REF_SHARED(refcnt, _Py_REF_MERGED);
413423
} while (!_Py_atomic_compare_exchange_ssize(&op->ob_ref_shared,
414424
&shared, new_shared));
415-
416-
#ifdef Py_REF_DEBUG
417-
_Py_AddRefTotal(_PyThreadState_GET(), extra);
418-
#endif
419-
420-
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
421-
_Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);
422425
return refcnt;
423426
}
424427
#endif /* Py_GIL_DISABLED */

0 commit comments

Comments
 (0)