Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
Description
Bug report
The_Py_MergeZeroLocalRefcount function is called when the local refcount field reaches zero. We generally maintain the invariant1 thatob_tid == 0 implies that the refcount fields are merged (i.e.,ob_ref_shared flags are_Py_REF_MERGED) and vice versa.
The current implementation breaks the invariant by settingob_tid to zero when the refcount fields aren't merged. Typically, this isn't a problem because:
- Most commonly, the object is deallocated so the values do not matter
- If the object is resurrected in
subtype_dealloc(e.g., for a finalizer), we usePy_SET_REFCNT, which will mark the fields as merged, restoring the invariant
However, if resurrection is done slightly differently, such as byPy_INCREF(), then things can break in very strange ways:
- The GC may restore
ob_tidfrom the allocator (because it's not merged), butob_ref_localis still zero. The nextPy_DECREFthen leads to a "negative refcount" error.
Summary
We should maintain the invariant thatob_tid == 0 <=>_Py_REF_IS_MERGED() and check it with assertions when possible.
Lines 373 to 398 in8303d32
| void | |
| _Py_MergeZeroLocalRefcount(PyObject*op) | |
| { | |
| assert(op->ob_ref_local==0); | |
| _Py_atomic_store_uintptr_relaxed(&op->ob_tid,0); | |
| Py_ssize_tshared=_Py_atomic_load_ssize_acquire(&op->ob_ref_shared); | |
| if (shared==0) { | |
| // Fast-path: shared refcount is zero (including flags) | |
| _Py_Dealloc(op); | |
| return; | |
| } | |
| // Slow-path: atomically set the flags (low two bits) to _Py_REF_MERGED. | |
| Py_ssize_tnew_shared; | |
| do { | |
| new_shared= (shared& ~_Py_REF_SHARED_FLAG_MASK) |_Py_REF_MERGED; | |
| }while (!_Py_atomic_compare_exchange_ssize(&op->ob_ref_shared, | |
| &shared,new_shared)); | |
| if (new_shared==_Py_REF_MERGED) { | |
| // i.e., the shared refcount is zero (only the flags are set) so we | |
| // deallocate the object. | |
| _Py_Dealloc(op); | |
| } | |
| } |
Originally reported by@albanD
Linked PRs
- gh-121794: Don't set
ob_tidto zero in fast-path dealloc #121799 - [3.13] gh-121794: Don't set
ob_tidto zero in fast-path dealloc (GH-121799) #121821
Footnotes
There are a few places where we deliberately re-use
ob_tidfor other purposes, such as the trashcan mechanism and during GC, but these objects are not visible to other parts of the program.↩