Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 8, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bdbad0
gh-117439: Make refleak checking thread-safe without the GIL
colesbury Apr 2, 2024
e3d02ca
Use relaxed atomic to avoid undefined behavior
colesbury Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Include/internal/pycore_object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalRefcountErrorFunc(
built against the pre-3.12 stable ABI. */
PyAPI_DATA(Py_ssize_t) _Py_RefTotal;

extern void _Py_AddRefTotal(PyInterpreterState *, Py_ssize_t);
extern void _Py_IncRefTotal(PyInterpreterState *);
extern void _Py_DecRefTotal(PyInterpreterState *);
extern void _Py_AddRefTotal(PyThreadState *, Py_ssize_t);
extern void _Py_IncRefTotal(PyThreadState *);
extern void _Py_DecRefTotal(PyThreadState *);

# define _Py_DEC_REFTOTAL(interp) \
interp->object_state.reftotal--
Expand All @@ -101,7 +101,7 @@ static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n)
return;
}
#ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), n);
_Py_AddRefTotal(_PyThreadState_GET(), n);
#endif
#if !defined(Py_GIL_DISABLED)
op->ob_refcnt += n;
Expand Down Expand Up @@ -394,7 +394,7 @@ _Py_TryIncrefFast(PyObject *op) {
_Py_INCREF_STAT_INC();
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local);
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
return 1;
}
Expand All @@ -417,7 +417,7 @@ _Py_TryIncRefShared(PyObject *op)
&shared,
shared + (1 << _Py_REF_SHARED_SHIFT))) {
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
_Py_INCREF_STAT_INC();
return 1;
Expand Down
4 changes: 4 additions & 0 deletions Include/internal/pycore_tstate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ typedef struct _PyThreadStateImpl {
struct _brc_thread_state brc;
#endif

#if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
Py_ssize_t reftotal; // this thread's total refcount operations
#endif

} _PyThreadStateImpl;


Expand Down
2 changes: 1 addition & 1 deletion Objects/bytesobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3084,7 +3084,7 @@ _PyBytes_Resize(PyObject **pv, Py_ssize_t newsize)
PyObject_Realloc(v, PyBytesObject_SIZE + newsize);
if (*pv == NULL) {
#ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET());
_Py_DecRefTotal(_PyThreadState_GET());
#endif
PyObject_Free(v);
PyErr_NoMemory();
Expand Down
10 changes: 5 additions & 5 deletions Objects/dictobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ dictkeys_incref(PyDictKeysObject *dk)
return;
}
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
INCREF_KEYS(dk);
}
Expand All @@ -454,7 +454,7 @@ dictkeys_decref(PyInterpreterState *interp, PyDictKeysObject *dk, bool use_qsbr)
}
assert(dk->dk_refcnt > 0);
#ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET());
_Py_DecRefTotal(_PyThreadState_GET());
#endif
if (DECREF_KEYS(dk) == 1) {
if (DK_IS_UNICODE(dk)) {
Expand Down Expand Up @@ -781,7 +781,7 @@ new_keys_object(PyInterpreterState *interp, uint8_t log2_size, bool unicode)
}
}
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
dk->dk_refcnt = 1;
dk->dk_log2_size = log2_size;
Expand Down Expand Up @@ -977,7 +977,7 @@ clone_combined_dict_keys(PyDictObject *orig)
we have it now; calling dictkeys_incref would be an error as
keys->dk_refcnt is already set to 1 (after memcpy). */
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
return keys;
}
Expand Down Expand Up @@ -2002,7 +2002,7 @@ dictresize(PyInterpreterState *interp, PyDictObject *mp,

if (oldkeys != Py_EMPTY_KEYS) {
#ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET());
_Py_DecRefTotal(_PyThreadState_GET());
#endif
assert(oldkeys->dk_kind != DICT_KEYS_SPLIT);
assert(oldkeys->dk_refcnt == 1);
Expand Down
62 changes: 34 additions & 28 deletions Objects/object.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,16 @@ get_legacy_reftotal(void)
interp->object_state.reftotal

static inline void
reftotal_increment(PyInterpreterState *interp)
reftotal_add(PyThreadState *tstate, Py_ssize_t n)
{
REFTOTAL(interp)++;
}

static inline void
reftotal_decrement(PyInterpreterState *interp)
{
REFTOTAL(interp)--;
}

static inline void
reftotal_add(PyInterpreterState *interp, Py_ssize_t n)
{
REFTOTAL(interp) += n;
#ifdef Py_GIL_DISABLED
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
// relaxed store to avoid data race with read in get_reftotal()
Py_ssize_t reftotal = tstate_impl->reftotal + n;
_Py_atomic_store_ssize_relaxed(&tstate_impl->reftotal, reftotal);
#else
REFTOTAL(tstate->interp) += n;
#endif
}

static inline Py_ssize_t get_global_reftotal(_PyRuntimeState *);
Expand Down Expand Up @@ -117,7 +112,15 @@ get_reftotal(PyInterpreterState *interp)
{
/* For a single interpreter, we ignore the legacy _Py_RefTotal,
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
Expand All @@ -129,7 +132,7 @@ get_global_reftotal(_PyRuntimeState *runtime)
HEAD_LOCK(&_PyRuntime);
PyInterpreterState *interp = PyInterpreterState_Head();
for (; interp != NULL; interp = PyInterpreterState_Next(interp)) {
total += REFTOTAL(interp);
total += get_reftotal(interp);
}
HEAD_UNLOCK(&_PyRuntime);

Expand Down Expand Up @@ -222,32 +225,32 @@ _Py_NegativeRefcount(const char *filename, int lineno, PyObject *op)
void
_Py_INCREF_IncRefTotal(void)
{
reftotal_increment(_PyInterpreterState_GET());
reftotal_add(_PyThreadState_GET(), 1);
}

/* This is used strictly by Py_DECREF(). */
void
_Py_DECREF_DecRefTotal(void)
{
reftotal_decrement(_PyInterpreterState_GET());
reftotal_add(_PyThreadState_GET(), -1);
}

void
_Py_IncRefTotal(PyInterpreterState *interp)
_Py_IncRefTotal(PyThreadState *tstate)
{
reftotal_increment(interp);
reftotal_add(tstate, 1);
}

void
_Py_DecRefTotal(PyInterpreterState *interp)
_Py_DecRefTotal(PyThreadState *tstate)
{
reftotal_decrement(interp);
reftotal_add(tstate, -1);
}

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
Expand All @@ -267,7 +270,10 @@ _Py_GetLegacyRefTotal(void)
Py_ssize_t
_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 */
Expand Down Expand Up @@ -345,7 +351,7 @@ _Py_DecRefSharedDebug(PyObject *o, const char *filename, int lineno)

if (should_queue) {
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
_Py_brc_queue_object(o);
}
Expand Down Expand Up @@ -405,7 +411,7 @@ _Py_ExplicitMergeRefcount(PyObject *op, Py_ssize_t extra)
&shared, new_shared));

#ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), extra);
_Py_AddRefTotal(_PyThreadState_GET(), extra);
#endif

_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
Expand Down Expand Up @@ -2388,7 +2394,7 @@ void
_Py_NewReference(PyObject *op)
{
#ifdef Py_REF_DEBUG
reftotal_increment(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
new_reference(op);
}
Expand Down
2 changes: 1 addition & 1 deletion Objects/tupleobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ _PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
if (sv == NULL) {
*pv = NULL;
#ifdef Py_REF_DEBUG
_Py_DecRefTotal(_PyInterpreterState_GET());
_Py_DecRefTotal(_PyThreadState_GET());
#endif
PyObject_GC_Del(v);
return -1;
Expand Down
2 changes: 1 addition & 1 deletion Objects/unicodeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14977,7 +14977,7 @@ _PyUnicode_InternInPlace(PyInterpreterState *interp, PyObject **p)
decrements to these objects will not be registered so they
need to be accounted for in here. */
for (Py_ssize_t i = 0; i < Py_REFCNT(s) - 2; i++) {
_Py_DecRefTotal(_PyInterpreterState_GET());
_Py_DecRefTotal(_PyThreadState_GET());
}
#endif
_Py_SetImmortal(s);
Expand Down
4 changes: 2 additions & 2 deletions Python/gc_free_threading.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ merge_refcount(PyObject *op, Py_ssize_t extra)
refcount += extra;

#ifdef Py_REF_DEBUG
_Py_AddRefTotal(_PyInterpreterState_GET(), extra);
_Py_AddRefTotal(_PyThreadState_GET(), extra);
#endif

// No atomics necessary; all other threads in this interpreter are paused.
Expand Down Expand Up @@ -307,7 +307,7 @@ merge_queued_objects(_PyThreadStateImpl *tstate, struct collection_state *state)
// decref and deallocate the object once we start the world again.
op->ob_ref_shared += (1 << _Py_REF_SHARED_SHIFT);
#ifdef Py_REF_DEBUG
_Py_IncRefTotal(_PyInterpreterState_GET());
_Py_IncRefTotal(_PyThreadState_GET());
#endif
worklist_push(&state->objs_to_decref, op);
}
Expand Down
8 changes: 8 additions & 0 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1689,6 +1689,14 @@ tstate_delete_common(PyThreadState *tstate)
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);

#ifdef Py_GIL_DISABLED
Expand Down
Loading