gh-117439: Make refleak checking thread-safe without the GIL (#117469)

This keeps track of the per-thread total reference count operations in
PyThreadState in the free-threaded builds. The count is merged into the
interpreter's total when the thread exits.
This commit is contained in:
Sam Gross 2024-04-08 12:11:36 -04:00 committed by GitHub
parent 2067da2579
commit 1a6594f661
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 62 additions and 44 deletions

View File

@ -86,9 +86,9 @@ PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalRefcountErrorFunc(
built against the pre-3.12 stable ABI. */ built against the pre-3.12 stable ABI. */
PyAPI_DATA(Py_ssize_t) _Py_RefTotal; PyAPI_DATA(Py_ssize_t) _Py_RefTotal;
extern void _Py_AddRefTotal(PyInterpreterState *, Py_ssize_t); extern void _Py_AddRefTotal(PyThreadState *, Py_ssize_t);
extern void _Py_IncRefTotal(PyInterpreterState *); extern void _Py_IncRefTotal(PyThreadState *);
extern void _Py_DecRefTotal(PyInterpreterState *); extern void _Py_DecRefTotal(PyThreadState *);
# define _Py_DEC_REFTOTAL(interp) \ # define _Py_DEC_REFTOTAL(interp) \
interp->object_state.reftotal-- interp->object_state.reftotal--
@ -101,7 +101,7 @@ static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n)
return; return;
} }
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), n); _Py_AddRefTotal(_PyThreadState_GET(), n);
#endif #endif
#if !defined(Py_GIL_DISABLED) #if !defined(Py_GIL_DISABLED)
op->ob_refcnt += n; op->ob_refcnt += n;
@ -393,7 +393,7 @@ _Py_TryIncrefFast(PyObject *op) {
_Py_INCREF_STAT_INC(); _Py_INCREF_STAT_INC();
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local); _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local);
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
return 1; return 1;
} }
@ -416,7 +416,7 @@ _Py_TryIncRefShared(PyObject *op)
&shared, &shared,
shared + (1 << _Py_REF_SHARED_SHIFT))) { shared + (1 << _Py_REF_SHARED_SHIFT))) {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
_Py_INCREF_STAT_INC(); _Py_INCREF_STAT_INC();
return 1; return 1;

View File

@ -38,6 +38,10 @@ typedef struct _PyThreadStateImpl {
struct _brc_thread_state brc; struct _brc_thread_state brc;
#endif #endif
#if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
Py_ssize_t reftotal; // this thread's total refcount operations
#endif
} _PyThreadStateImpl; } _PyThreadStateImpl;

View File

@ -3118,7 +3118,7 @@ _PyBytes_Resize(PyObject **pv, Py_ssize_t newsize)
PyObject_Realloc(v, PyBytesObject_SIZE + newsize); PyObject_Realloc(v, PyBytesObject_SIZE + newsize);
if (*pv == NULL) { if (*pv == NULL) {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET()); _Py_DecRefTotal(_PyThreadState_GET());
#endif #endif
PyObject_Free(v); PyObject_Free(v);
PyErr_NoMemory(); PyErr_NoMemory();

View File

@ -445,7 +445,7 @@ dictkeys_incref(PyDictKeysObject *dk)
return; return;
} }
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
INCREF_KEYS(dk); INCREF_KEYS(dk);
} }
@ -458,7 +458,7 @@ dictkeys_decref(PyInterpreterState *interp, PyDictKeysObject *dk, bool use_qsbr)
} }
assert(dk->dk_refcnt > 0); assert(dk->dk_refcnt > 0);
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET()); _Py_DecRefTotal(_PyThreadState_GET());
#endif #endif
if (DECREF_KEYS(dk) == 1) { if (DECREF_KEYS(dk) == 1) {
if (DK_IS_UNICODE(dk)) { if (DK_IS_UNICODE(dk)) {
@ -790,7 +790,7 @@ new_keys_object(PyInterpreterState *interp, uint8_t log2_size, bool unicode)
} }
} }
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
dk->dk_refcnt = 1; dk->dk_refcnt = 1;
dk->dk_log2_size = log2_size; dk->dk_log2_size = log2_size;
@ -978,7 +978,7 @@ clone_combined_dict_keys(PyDictObject *orig)
we have it now; calling dictkeys_incref would be an error as we have it now; calling dictkeys_incref would be an error as
keys->dk_refcnt is already set to 1 (after memcpy). */ keys->dk_refcnt is already set to 1 (after memcpy). */
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
return keys; return keys;
} }
@ -2021,7 +2021,7 @@ dictresize(PyInterpreterState *interp, PyDictObject *mp,
if (oldkeys != Py_EMPTY_KEYS) { if (oldkeys != Py_EMPTY_KEYS) {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET()); _Py_DecRefTotal(_PyThreadState_GET());
#endif #endif
assert(oldkeys->dk_kind != DICT_KEYS_SPLIT); assert(oldkeys->dk_kind != DICT_KEYS_SPLIT);
assert(oldkeys->dk_refcnt == 1); assert(oldkeys->dk_refcnt == 1);

View File

@ -73,21 +73,16 @@ get_legacy_reftotal(void)
interp->object_state.reftotal interp->object_state.reftotal
static inline void static inline void
reftotal_increment(PyInterpreterState *interp) reftotal_add(PyThreadState *tstate, Py_ssize_t n)
{ {
REFTOTAL(interp)++; #ifdef Py_GIL_DISABLED
} _PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
// relaxed store to avoid data race with read in get_reftotal()
static inline void Py_ssize_t reftotal = tstate_impl->reftotal + n;
reftotal_decrement(PyInterpreterState *interp) _Py_atomic_store_ssize_relaxed(&tstate_impl->reftotal, reftotal);
{ #else
REFTOTAL(interp)--; REFTOTAL(tstate->interp) += n;
} #endif
static inline void
reftotal_add(PyInterpreterState *interp, Py_ssize_t n)
{
REFTOTAL(interp) += n;
} }
static inline Py_ssize_t get_global_reftotal(_PyRuntimeState *); static inline Py_ssize_t get_global_reftotal(_PyRuntimeState *);
@ -117,7 +112,15 @@ get_reftotal(PyInterpreterState *interp)
{ {
/* For a single interpreter, we ignore the legacy _Py_RefTotal, /* For a single interpreter, we ignore the legacy _Py_RefTotal,
since we can't determine which interpreter updated it. */ since we can't determine which interpreter updated it. */
return REFTOTAL(interp); Py_ssize_t total = REFTOTAL(interp);
#ifdef Py_GIL_DISABLED
for (PyThreadState *p = interp->threads.head; p != NULL; p = p->next) {
/* This may race with other threads modifications to their reftotal */
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)p;
total += _Py_atomic_load_ssize_relaxed(&tstate_impl->reftotal);
}
#endif
return total;
} }
static inline Py_ssize_t static inline Py_ssize_t
@ -129,7 +132,7 @@ get_global_reftotal(_PyRuntimeState *runtime)
HEAD_LOCK(&_PyRuntime); HEAD_LOCK(&_PyRuntime);
PyInterpreterState *interp = PyInterpreterState_Head(); PyInterpreterState *interp = PyInterpreterState_Head();
for (; interp != NULL; interp = PyInterpreterState_Next(interp)) { for (; interp != NULL; interp = PyInterpreterState_Next(interp)) {
total += REFTOTAL(interp); total += get_reftotal(interp);
} }
HEAD_UNLOCK(&_PyRuntime); HEAD_UNLOCK(&_PyRuntime);
@ -222,32 +225,32 @@ _Py_NegativeRefcount(const char *filename, int lineno, PyObject *op)
void void
_Py_INCREF_IncRefTotal(void) _Py_INCREF_IncRefTotal(void)
{ {
reftotal_increment(_PyInterpreterState_GET()); reftotal_add(_PyThreadState_GET(), 1);
} }
/* This is used strictly by Py_DECREF(). */ /* This is used strictly by Py_DECREF(). */
void void
_Py_DECREF_DecRefTotal(void) _Py_DECREF_DecRefTotal(void)
{ {
reftotal_decrement(_PyInterpreterState_GET()); reftotal_add(_PyThreadState_GET(), -1);
} }
void void
_Py_IncRefTotal(PyInterpreterState *interp) _Py_IncRefTotal(PyThreadState *tstate)
{ {
reftotal_increment(interp); reftotal_add(tstate, 1);
} }
void void
_Py_DecRefTotal(PyInterpreterState *interp) _Py_DecRefTotal(PyThreadState *tstate)
{ {
reftotal_decrement(interp); reftotal_add(tstate, -1);
} }
void void
_Py_AddRefTotal(PyInterpreterState *interp, Py_ssize_t n) _Py_AddRefTotal(PyThreadState *tstate, Py_ssize_t n)
{ {
reftotal_add(interp, n); reftotal_add(tstate, n);
} }
/* This includes the legacy total /* This includes the legacy total
@ -267,7 +270,10 @@ _Py_GetLegacyRefTotal(void)
Py_ssize_t Py_ssize_t
_PyInterpreterState_GetRefTotal(PyInterpreterState *interp) _PyInterpreterState_GetRefTotal(PyInterpreterState *interp)
{ {
return get_reftotal(interp); HEAD_LOCK(&_PyRuntime);
Py_ssize_t total = get_reftotal(interp);
HEAD_UNLOCK(&_PyRuntime);
return total;
} }
#endif /* Py_REF_DEBUG */ #endif /* Py_REF_DEBUG */
@ -345,7 +351,7 @@ _Py_DecRefSharedDebug(PyObject *o, const char *filename, int lineno)
if (should_queue) { if (should_queue) {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
_Py_brc_queue_object(o); _Py_brc_queue_object(o);
} }
@ -405,7 +411,7 @@ _Py_ExplicitMergeRefcount(PyObject *op, Py_ssize_t extra)
&shared, new_shared)); &shared, new_shared));
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), extra); _Py_AddRefTotal(_PyThreadState_GET(), extra);
#endif #endif
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0); _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
@ -2376,7 +2382,7 @@ void
_Py_NewReference(PyObject *op) _Py_NewReference(PyObject *op)
{ {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
reftotal_increment(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
new_reference(op); new_reference(op);
} }

View File

@ -946,7 +946,7 @@ _PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
if (sv == NULL) { if (sv == NULL) {
*pv = NULL; *pv = NULL;
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET()); _Py_DecRefTotal(_PyThreadState_GET());
#endif #endif
PyObject_GC_Del(v); PyObject_GC_Del(v);
return -1; return -1;

View File

@ -14916,7 +14916,7 @@ _PyUnicode_InternInPlace(PyInterpreterState *interp, PyObject **p)
decrements to these objects will not be registered so they decrements to these objects will not be registered so they
need to be accounted for in here. */ need to be accounted for in here. */
for (Py_ssize_t i = 0; i < Py_REFCNT(s) - 2; i++) { for (Py_ssize_t i = 0; i < Py_REFCNT(s) - 2; i++) {
_Py_DecRefTotal(_PyInterpreterState_GET()); _Py_DecRefTotal(_PyThreadState_GET());
} }
#endif #endif
_Py_SetImmortal(s); _Py_SetImmortal(s);

View File

@ -168,7 +168,7 @@ merge_refcount(PyObject *op, Py_ssize_t extra)
refcount += extra; refcount += extra;
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), extra); _Py_AddRefTotal(_PyThreadState_GET(), extra);
#endif #endif
// No atomics necessary; all other threads in this interpreter are paused. // No atomics necessary; all other threads in this interpreter are paused.
@ -307,7 +307,7 @@ merge_queued_objects(_PyThreadStateImpl *tstate, struct collection_state *state)
// decref and deallocate the object once we start the world again. // decref and deallocate the object once we start the world again.
op->ob_ref_shared += (1 << _Py_REF_SHARED_SHIFT); op->ob_ref_shared += (1 << _Py_REF_SHARED_SHIFT);
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET()); _Py_IncRefTotal(_PyThreadState_GET());
#endif #endif
worklist_push(&state->objs_to_decref, op); worklist_push(&state->objs_to_decref, op);
} }

View File

@ -1698,6 +1698,14 @@ tstate_delete_common(PyThreadState *tstate)
decrement_stoptheworld_countdown(&runtime->stoptheworld); decrement_stoptheworld_countdown(&runtime->stoptheworld);
} }
} }
#if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
// Add our portion of the total refcount to the interpreter's total.
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
tstate->interp->object_state.reftotal += tstate_impl->reftotal;
tstate_impl->reftotal = 0;
#endif
HEAD_UNLOCK(runtime); HEAD_UNLOCK(runtime);
#ifdef Py_GIL_DISABLED #ifdef Py_GIL_DISABLED