[Python-checkins] gh-102304: Move the Total Refcount to PyInterpreterState (gh-102545)
ericsnowcurrently
webhook-mailer at python.org
Tue Mar 21 13:46:18 EDT 2023
https://github.com/python/cpython/commit/743687434c5baf01c266320b34c7a828726702a6
commit: 743687434c5baf01c266320b34c7a828726702a6
branch: main
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: ericsnowcurrently <ericsnowcurrently at gmail.com>
date: 2023-03-21T11:46:09-06:00
summary:
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
files:
M Include/cpython/object.h
M Include/internal/pycore_interp.h
M Include/internal/pycore_object.h
M Include/internal/pycore_object_state.h
M Objects/bytesobject.c
M Objects/dictobject.c
M Objects/object.c
M Objects/structseq.c
M Objects/tupleobject.c
M Objects/typeobject.c
M Python/pylifecycle.c
M Python/pystate.c
M Python/sysmodule.c
diff --git a/Include/cpython/object.h b/Include/cpython/object.h
index 0438612edd1d..859ffb91e223 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 84303318d218..1f2c0db2eb5f 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 b985eff8a8a0..d6bbafd4b6cc 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 4e5862a11edd..94005d778814 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 687a654bdae1..2d8dab6f3780 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 53f9a380346a..2ef520044340 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 95f7c966a414..9dd5eb998217 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 c20962ecd825..2a5343815866 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 59c0251639d3..61fab4078d66 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 f0654c239f66..a37f97c71ec7 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 8b58a14c693f..0d546d52087e 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 60adb54685ce..b17efdbefd12 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 20761738b527..4afb0f1d0b5e 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();
}
More information about the Python-checkins
mailing list