Skip to content

Commit 1bf07dc

Browse files
committed
Fix decref of old attribute value
1 parent f650d7e commit 1bf07dc

1 file changed

Lines changed: 16 additions & 16 deletions

File tree

mypyc/lib-rt/pythonsupport.h

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,26 +97,26 @@ static inline PyObject *CPy_GetAttrRefFinal(PyObject **field) {
9797

9898
// Reclaim the previous value of a native attribute after it has been replaced.
9999
//
100-
// The fast path avoids QSBR deferral when the decref provably cannot free the
101-
// object: immortal objects are never freed, and an object owned by the current
102-
// thread with a local refcount > 1 stays at refcount >= 1 after a plain local
103-
// decrement, so no concurrent reader can ever observe a freed header. Only the
104-
// cases that could actually free (local refcount would reach zero, or the object
105-
// is owned by another thread) defer the decref via QSBR, which keeps the memory
106-
// valid until every thread has passed a quiescent point -- long enough for an
107-
// in-flight reader that loaded the old pointer to finish its try-incref.
100+
// CPy_GetAttrRef reads the field optimistically without holding any lock the
101+
// writer also takes, so a reader can load the old pointer and then dereference it
102+
// (in its try-incref / re-validation) after this store. The old value must
103+
// therefore stay alive until every thread has passed a quiescent point, which is
104+
// exactly what a QSBR-deferred decref guarantees. So all mortal old values are
105+
// reclaimed via _PyObject_XDecRefDelayed, matching CPython's own replace-a-slot
106+
// paths (e.g. _PyObject_SetDict / _PyObject_SetManagedDict).
107+
//
108+
// We deliberately do NOT take a "local refcount > 1, owned by this thread" fast
109+
// path: dropping the field's reference is not the only decref of the object, so a
110+
// non-freeing local decrement here does not prevent an unrelated reference holder
111+
// from driving the object to zero (a plain, non-deferred Py_DECREF -> _Py_Dealloc)
112+
// while an in-flight reader still holds the stale pointer -- a use-after-free that
113+
// only QSBR deferral closes. Immortal objects are never freed, so skipping their
114+
// decref entirely is safe and avoids queuing a no-op onto the delayed-free list.
108115
static inline void CPy_DecRefAttrOld(PyObject *op) {
109116
if (op == NULL) {
110117
return;
111118
}
112-
uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local);
113-
if (local == _Py_IMMORTAL_REFCNT_LOCAL) {
114-
return;
115-
}
116-
if (local > 1 && _Py_IsOwnedByCurrentThread(op)) {
117-
// Same as Py_DECREF's owned-thread fast path: only this thread writes
118-
// ob_ref_local, and the refcount stays >= 1, so this is race-free.
119-
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local - 1);
119+
if (_Py_IsImmortal(op)) {
120120
return;
121121
}
122122
_PyObject_XDecRefDelayed(op);

0 commit comments

Comments
 (0)