From 743687434c5baf01c266320b34c7a828726702a6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 21 Mar 2023 11:46:09 -0600 Subject: [PATCH] gh-102304: Move the Total Refcount to PyInterpreterState (gh-102545) Moving it valuable with a per-interpreter GIL. However, it is also useful without one, since it allows us to identify refleaks within a single interpreter or where references are escaping an interpreter. This becomes more important as we move the obmalloc state to PyInterpreterState. https://github.com/python/cpython/issues/102304 --- Include/cpython/object.h | 1 + Include/internal/pycore_interp.h | 2 + Include/internal/pycore_object.h | 16 +++--- Include/internal/pycore_object_state.h | 8 +++ Objects/bytesobject.c | 2 +- Objects/dictobject.c | 10 ++-- Objects/object.c | 79 +++++++++++++++++++------- Objects/structseq.c | 2 +- Objects/tupleobject.c | 2 +- Objects/typeobject.c | 18 +++++- Python/pylifecycle.c | 5 ++ Python/pystate.c | 10 +++- Python/sysmodule.c | 2 + 13 files changed, 117 insertions(+), 40 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 0438612edd1..859ffb91e22 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -15,6 +15,7 @@ PyAPI_FUNC(void) _Py_ForgetReference(PyObject *); PyAPI_FUNC(Py_ssize_t) _Py_GetGlobalRefTotal(void); # define _Py_GetRefTotal() _Py_GetGlobalRefTotal() PyAPI_FUNC(Py_ssize_t) _Py_GetLegacyRefTotal(void); +PyAPI_FUNC(Py_ssize_t) _PyInterpreterState_GetRefTotal(PyInterpreterState *); #endif diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 84303318d21..1f2c0db2eb5 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -25,6 +25,7 @@ extern "C" { #include "pycore_import.h" // struct _import_state #include "pycore_list.h" // struct _Py_list_state #include "pycore_global_objects.h" // struct _Py_interp_static_objects +#include "pycore_object_state.h" // struct _py_object_state #include "pycore_tuple.h" // struct _Py_tuple_state #include "pycore_typeobject.h" // struct type_cache #include "pycore_unicodeobject.h" // struct _Py_unicode_state @@ -138,6 +139,7 @@ struct _is { // One bit is set for each non-NULL entry in code_watchers uint8_t active_code_watchers; + struct _py_object_state object_state; struct _Py_unicode_state unicode; struct _Py_float_state float_state; struct _Py_long_state long_state; diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index b985eff8a8a..d6bbafd4b6c 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -43,18 +43,19 @@ 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(Py_ssize_t); -extern void _Py_IncRefTotal(void); -extern void _Py_DecRefTotal(void); +extern void _Py_AddRefTotal(PyInterpreterState *, Py_ssize_t); +extern void _Py_IncRefTotal(PyInterpreterState *); +extern void _Py_DecRefTotal(PyInterpreterState *); -# define _Py_DEC_REFTOTAL() _PyRuntime.object_state.reftotal-- +# define _Py_DEC_REFTOTAL(interp) \ + interp->object_state.reftotal-- #endif // Increment reference count by n static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n) { #ifdef Py_REF_DEBUG - _Py_AddRefTotal(n); + _Py_AddRefTotal(_PyInterpreterState_GET(), n); #endif op->ob_refcnt += n; } @@ -65,7 +66,7 @@ _Py_DECREF_SPECIALIZED(PyObject *op, const destructor destruct) { _Py_DECREF_STAT_INC(); #ifdef Py_REF_DEBUG - _Py_DEC_REFTOTAL(); + _Py_DEC_REFTOTAL(_PyInterpreterState_GET()); #endif if (--op->ob_refcnt != 0) { assert(op->ob_refcnt > 0); @@ -83,7 +84,7 @@ _Py_DECREF_NO_DEALLOC(PyObject *op) { _Py_DECREF_STAT_INC(); #ifdef Py_REF_DEBUG - _Py_DEC_REFTOTAL(); + _Py_DEC_REFTOTAL(_PyInterpreterState_GET()); #endif op->ob_refcnt--; #ifdef Py_DEBUG @@ -226,6 +227,7 @@ static inline void _PyObject_GC_UNTRACK( #endif #ifdef Py_REF_DEBUG +extern void _PyInterpreterState_FinalizeRefTotal(PyInterpreterState *); extern void _Py_FinalizeRefTotal(_PyRuntimeState *); extern void _PyDebug_PrintTotalRefs(void); #endif diff --git a/Include/internal/pycore_object_state.h b/Include/internal/pycore_object_state.h index 4e5862a11ed..94005d77881 100644 --- a/Include/internal/pycore_object_state.h +++ b/Include/internal/pycore_object_state.h @@ -9,6 +9,14 @@ extern "C" { #endif struct _py_object_runtime_state { +#ifdef Py_REF_DEBUG + Py_ssize_t interpreter_leaks; +#else + int _not_used; +#endif +}; + +struct _py_object_state { #ifdef Py_REF_DEBUG Py_ssize_t reftotal; #else diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 687a654bdae..2d8dab6f378 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3067,7 +3067,7 @@ _PyBytes_Resize(PyObject **pv, Py_ssize_t newsize) PyObject_Realloc(v, PyBytesObject_SIZE + newsize); if (*pv == NULL) { #ifdef Py_REF_DEBUG - _Py_DecRefTotal(); + _Py_DecRefTotal(_PyInterpreterState_GET()); #endif PyObject_Free(v); PyErr_NoMemory(); diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 53f9a380346..2ef52004434 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -304,7 +304,7 @@ static inline void dictkeys_incref(PyDictKeysObject *dk) { #ifdef Py_REF_DEBUG - _Py_IncRefTotal(); + _Py_IncRefTotal(_PyInterpreterState_GET()); #endif dk->dk_refcnt++; } @@ -314,7 +314,7 @@ dictkeys_decref(PyInterpreterState *interp, PyDictKeysObject *dk) { assert(dk->dk_refcnt > 0); #ifdef Py_REF_DEBUG - _Py_DecRefTotal(); + _Py_DecRefTotal(_PyInterpreterState_GET()); #endif if (--dk->dk_refcnt == 0) { free_keys_object(interp, dk); @@ -634,7 +634,7 @@ new_keys_object(PyInterpreterState *interp, uint8_t log2_size, bool unicode) } } #ifdef Py_REF_DEBUG - _Py_IncRefTotal(); + _Py_IncRefTotal(_PyInterpreterState_GET()); #endif dk->dk_refcnt = 1; dk->dk_log2_size = log2_size; @@ -824,7 +824,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(); + _Py_IncRefTotal(_PyInterpreterState_GET()); #endif return keys; } @@ -1530,7 +1530,7 @@ dictresize(PyInterpreterState *interp, PyDictObject *mp, // We can not use free_keys_object here because key's reference // are moved already. #ifdef Py_REF_DEBUG - _Py_DecRefTotal(); + _Py_DecRefTotal(_PyInterpreterState_GET()); #endif if (oldkeys == Py_EMPTY_KEYS) { oldkeys->dk_refcnt--; diff --git a/Objects/object.c b/Objects/object.c index 95f7c966a41..9dd5eb99821 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -66,25 +66,25 @@ get_legacy_reftotal(void) #ifdef Py_REF_DEBUG -# define REFTOTAL(runtime) \ - (runtime)->object_state.reftotal +# define REFTOTAL(interp) \ + interp->object_state.reftotal static inline void -reftotal_increment(_PyRuntimeState *runtime) +reftotal_increment(PyInterpreterState *interp) { - REFTOTAL(runtime)++; + REFTOTAL(interp)++; } static inline void -reftotal_decrement(_PyRuntimeState *runtime) +reftotal_decrement(PyInterpreterState *interp) { - REFTOTAL(runtime)--; + REFTOTAL(interp)--; } static inline void -reftotal_add(_PyRuntimeState *runtime, Py_ssize_t n) +reftotal_add(PyInterpreterState *interp, Py_ssize_t n) { - REFTOTAL(runtime) += n; + REFTOTAL(interp) += n; } static inline Py_ssize_t get_global_reftotal(_PyRuntimeState *); @@ -99,15 +99,43 @@ void _Py_FinalizeRefTotal(_PyRuntimeState *runtime) { last_final_reftotal = get_global_reftotal(runtime); - REFTOTAL(runtime) = 0; + runtime->object_state.interpreter_leaks = 0; +} + +void +_PyInterpreterState_FinalizeRefTotal(PyInterpreterState *interp) +{ + interp->runtime->object_state.interpreter_leaks += REFTOTAL(interp); + REFTOTAL(interp) = 0; +} + +static inline Py_ssize_t +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); } static inline Py_ssize_t get_global_reftotal(_PyRuntimeState *runtime) { - /* For an update from _Py_RefTotal first. */ - Py_ssize_t legacy = get_legacy_reftotal(); - return REFTOTAL(runtime) + legacy + last_final_reftotal; + Py_ssize_t total = 0; + + /* Add up the total from each interpreter. */ + HEAD_LOCK(&_PyRuntime); + PyInterpreterState *interp = PyInterpreterState_Head(); + for (; interp != NULL; interp = PyInterpreterState_Next(interp)) { + total += REFTOTAL(interp); + } + HEAD_UNLOCK(&_PyRuntime); + + /* Add in the updated value from the legacy _Py_RefTotal. */ + total += get_legacy_reftotal(); + total += last_final_reftotal; + total += runtime->object_state.interpreter_leaks; + + return total; } #undef REFTOTAL @@ -118,7 +146,8 @@ _PyDebug_PrintTotalRefs(void) { fprintf(stderr, "[%zd refs, %zd blocks]\n", get_global_reftotal(runtime), _Py_GetAllocatedBlocks()); - /* It may be helpful to also print the "legacy" reftotal separately. */ + /* It may be helpful to also print the "legacy" reftotal separately. + Likewise for the total for each interpreter. */ } #endif /* Py_REF_DEBUG */ @@ -177,32 +206,32 @@ _Py_NegativeRefcount(const char *filename, int lineno, PyObject *op) void _Py_IncRefTotal_DO_NOT_USE_THIS(void) { - reftotal_increment(&_PyRuntime); + reftotal_increment(_PyInterpreterState_GET()); } /* This is used strictly by Py_DECREF(). */ void _Py_DecRefTotal_DO_NOT_USE_THIS(void) { - reftotal_decrement(&_PyRuntime); + reftotal_decrement(_PyInterpreterState_GET()); } void -_Py_IncRefTotal(void) +_Py_IncRefTotal(PyInterpreterState *interp) { - reftotal_increment(&_PyRuntime); + reftotal_increment(interp); } void -_Py_DecRefTotal(void) +_Py_DecRefTotal(PyInterpreterState *interp) { - reftotal_decrement(&_PyRuntime); + reftotal_decrement(interp); } void -_Py_AddRefTotal(Py_ssize_t n) +_Py_AddRefTotal(PyInterpreterState *interp, Py_ssize_t n) { - reftotal_add(&_PyRuntime, n); + reftotal_add(interp, n); } /* This includes the legacy total @@ -219,6 +248,12 @@ _Py_GetLegacyRefTotal(void) return get_legacy_reftotal(); } +Py_ssize_t +_PyInterpreterState_GetRefTotal(PyInterpreterState *interp) +{ + return get_reftotal(interp); +} + #endif /* Py_REF_DEBUG */ void @@ -2128,7 +2163,7 @@ void _Py_NewReference(PyObject *op) { #ifdef Py_REF_DEBUG - reftotal_increment(&_PyRuntime); + reftotal_increment(_PyInterpreterState_GET()); #endif new_reference(op); } diff --git a/Objects/structseq.c b/Objects/structseq.c index c20962ecd82..2a534381586 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -592,7 +592,7 @@ _PyStructSequence_FiniType(PyTypeObject *type) // Don't use Py_DECREF(): static type must not be deallocated Py_SET_REFCNT(type, 0); #ifdef Py_REF_DEBUG - _Py_DecRefTotal(); + _Py_DecRefTotal(_PyInterpreterState_GET()); #endif // Make sure that _PyStructSequence_InitType() will initialize diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 59c0251639d..61fab4078d6 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -944,7 +944,7 @@ _PyTuple_Resize(PyObject **pv, Py_ssize_t newsize) if (sv == NULL) { *pv = NULL; #ifdef Py_REF_DEBUG - _Py_DecRefTotal(); + _Py_DecRefTotal(_PyInterpreterState_GET()); #endif PyObject_GC_Del(v); return -1; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index f0654c239f6..a37f97c71ec 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -317,11 +317,27 @@ _PyType_InitCache(PyInterpreterState *interp) entry->version = 0; // Set to None so _PyType_Lookup() can use Py_SETREF(), // rather than using slower Py_XSETREF(). - entry->name = Py_NewRef(Py_None); + // (See _PyType_FixCacheRefcounts() about the refcount.) + entry->name = Py_None; entry->value = NULL; } } +// This is the temporary fix used by pycore_create_interpreter(), +// in pylifecycle.c. _PyType_InitCache() is called before the GIL +// has been created (for the main interpreter) and without the +// "current" thread state set. This causes crashes when the +// reftotal is updated, so we don't modify the refcount in +// _PyType_InitCache(), and instead do it later by calling +// _PyType_FixCacheRefcounts(). +// XXX This workaround should be removed once we have immortal +// objects (PEP 683). +void +_PyType_FixCacheRefcounts(void) +{ + _Py_RefcntAdd(Py_None, (1 << MCACHE_SIZE_EXP)); +} + static unsigned int _PyType_ClearCache(PyInterpreterState *interp) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 8b58a14c693..0d546d52087 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -802,6 +802,11 @@ pycore_interp_init(PyThreadState *tstate) PyStatus status; PyObject *sysmod = NULL; + // This is a temporary fix until we have immortal objects. + // (See _PyType_InitCache() in typeobject.c.) + extern void _PyType_FixCacheRefcounts(void); + _PyType_FixCacheRefcounts(); + // Create singletons before the first PyType_Ready() call, since // PyType_Ready() uses singletons like the Unicode empty string (tp_doc) // and the empty tuple singletons (tp_bases). diff --git a/Python/pystate.c b/Python/pystate.c index 60adb54685c..b17efdbefd1 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -483,8 +483,8 @@ void _PyRuntimeState_Fini(_PyRuntimeState *runtime) { #ifdef Py_REF_DEBUG - /* The reftotal is cleared by _Py_FinalizeRefTotal(). */ - assert(runtime->object_state.reftotal == 0); + /* The count is cleared by _Py_FinalizeRefTotal(). */ + assert(runtime->object_state.interpreter_leaks == 0); #endif if (gilstate_tss_initialized(runtime)) { @@ -904,6 +904,12 @@ PyInterpreterState_Delete(PyInterpreterState *interp) _PyEval_FiniState(&interp->ceval); +#ifdef Py_REF_DEBUG + // XXX This call should be done at the end of clear_interpreter(), + // but currently some objects get decref'ed after that. + _PyInterpreterState_FinalizeRefTotal(interp); +#endif + HEAD_LOCK(runtime); PyInterpreterState **p; for (p = &interpreters->head; ; p = &(*p)->next) { diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 20761738b52..4afb0f1d0b5 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1854,6 +1854,8 @@ static Py_ssize_t sys_gettotalrefcount_impl(PyObject *module) /*[clinic end generated code: output=4103886cf17c25bc input=53b744faa5d2e4f6]*/ { + /* It may make sense to return the total for the current interpreter + or have a second function that does so. */ return _Py_GetGlobalRefTotal(); }