From ad77d16a6252c2e616bf41b981a6d919c1122b4d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 20 Mar 2023 10:03:04 -0600 Subject: [PATCH] gh-102304: Move _Py_RefTotal to _PyRuntimeState (gh-102543) The essentially eliminates the global variable, with the associated benefits. This is also a precursor to isolating this bit of state to PyInterpreterState. Folks that currently read _Py_RefTotal directly would have to start using _Py_GetGlobalRefTotal() instead. https://github.com/python/cpython/issues/102304 --- Include/cpython/object.h | 5 +- Include/internal/pycore_object.h | 4 +- Include/internal/pycore_object_state.h | 23 +++++ Include/internal/pycore_runtime.h | 2 + Include/object.h | 9 +- Makefile.pre.in | 1 + Objects/object.c | 123 +++++++++++++++++-------- PCbuild/pythoncore.vcxproj | 1 + PCbuild/pythoncore.vcxproj.filters | 3 + Python/pylifecycle.c | 1 + Python/pystate.c | 3 + Python/sysmodule.c | 2 +- Tools/c-analyzer/cpython/ignored.tsv | 2 +- 13 files changed, 132 insertions(+), 47 deletions(-) create mode 100644 Include/internal/pycore_object_state.h diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 7b687d31135..0438612edd1 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -11,7 +11,10 @@ PyAPI_FUNC(void) _Py_ForgetReference(PyObject *); #endif #ifdef Py_REF_DEBUG -PyAPI_FUNC(Py_ssize_t) _Py_GetRefTotal(void); +/* These are useful as debugging aids when chasing down refleaks. */ +PyAPI_FUNC(Py_ssize_t) _Py_GetGlobalRefTotal(void); +# define _Py_GetRefTotal() _Py_GetGlobalRefTotal() +PyAPI_FUNC(Py_ssize_t) _Py_GetLegacyRefTotal(void); #endif diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 318e6f3371c..b985eff8a8a 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -46,7 +46,8 @@ PyAPI_DATA(Py_ssize_t) _Py_RefTotal; extern void _Py_AddRefTotal(Py_ssize_t); extern void _Py_IncRefTotal(void); extern void _Py_DecRefTotal(void); -# define _Py_DEC_REFTOTAL() _Py_RefTotal-- + +# define _Py_DEC_REFTOTAL() _PyRuntime.object_state.reftotal-- #endif // Increment reference count by n @@ -225,6 +226,7 @@ static inline void _PyObject_GC_UNTRACK( #endif #ifdef Py_REF_DEBUG +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 new file mode 100644 index 00000000000..4e5862a11ed --- /dev/null +++ b/Include/internal/pycore_object_state.h @@ -0,0 +1,23 @@ +#ifndef Py_INTERNAL_OBJECT_STATE_H +#define Py_INTERNAL_OBJECT_STATE_H +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef Py_BUILD_CORE +# error "this header requires Py_BUILD_CORE define" +#endif + +struct _py_object_runtime_state { +#ifdef Py_REF_DEBUG + Py_ssize_t reftotal; +#else + int _not_used; +#endif +}; + + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_OBJECT_STATE_H */ diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 520109ca440..de757dfa93b 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -15,6 +15,7 @@ extern "C" { #include "pycore_global_objects.h" // struct _Py_global_objects #include "pycore_import.h" // struct _import_runtime_state #include "pycore_interp.h" // PyInterpreterState +#include "pycore_object_state.h" // struct _py_object_runtime_state #include "pycore_parser.h" // struct _parser_runtime_state #include "pycore_pymem.h" // struct _pymem_allocators #include "pycore_pyhash.h" // struct pyhash_runtime_state @@ -150,6 +151,7 @@ typedef struct pyruntimestate { void *open_code_userdata; _Py_AuditHookEntry *audit_hook_head; + struct _py_object_runtime_state object_state; struct _Py_float_runtime_state float_state; struct _Py_unicode_runtime_state unicode_state; diff --git a/Include/object.h b/Include/object.h index 844b9c4a51c..fc577353c1c 100644 --- a/Include/object.h +++ b/Include/object.h @@ -494,14 +494,9 @@ you can count such references to the type object.) extern Py_ssize_t _Py_RefTotal; # define _Py_INC_REFTOTAL() _Py_RefTotal++ # define _Py_DEC_REFTOTAL() _Py_RefTotal-- -# elif defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) -extern void _Py_IncRefTotal(void); -extern void _Py_DecRefTotal(void); -# define _Py_INC_REFTOTAL() _Py_IncRefTotal() -# define _Py_DEC_REFTOTAL() _Py_DecRefTotal() # elif !defined(Py_LIMITED_API) || Py_LIMITED_API+0 > 0x030C0000 -extern void _Py_IncRefTotal_DO_NOT_USE_THIS(void); -extern void _Py_DecRefTotal_DO_NOT_USE_THIS(void); +PyAPI_FUNC(void) _Py_IncRefTotal_DO_NOT_USE_THIS(void); +PyAPI_FUNC(void) _Py_DecRefTotal_DO_NOT_USE_THIS(void); # define _Py_INC_REFTOTAL() _Py_IncRefTotal_DO_NOT_USE_THIS() # define _Py_DEC_REFTOTAL() _Py_DecRefTotal_DO_NOT_USE_THIS() # endif diff --git a/Makefile.pre.in b/Makefile.pre.in index 8f13198e7e3..74e4171b010 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1699,6 +1699,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_moduleobject.h \ $(srcdir)/Include/internal/pycore_namespace.h \ $(srcdir)/Include/internal/pycore_object.h \ + $(srcdir)/Include/internal/pycore_object_state.h \ $(srcdir)/Include/internal/pycore_obmalloc.h \ $(srcdir)/Include/internal/pycore_obmalloc_init.h \ $(srcdir)/Include/internal/pycore_pathconfig.h \ diff --git a/Objects/object.c b/Objects/object.c index dff5e2afa16..95f7c966a41 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -54,37 +54,71 @@ _PyObject_CheckConsistency(PyObject *op, int check_content) #ifdef Py_REF_DEBUG +/* We keep the legacy symbol around for backward compatibility. */ Py_ssize_t _Py_RefTotal; -static inline void -reftotal_increment(void) -{ - _Py_RefTotal++; -} - -static inline void -reftotal_decrement(void) -{ - _Py_RefTotal--; -} - -void -_Py_AddRefTotal(Py_ssize_t n) -{ - _Py_RefTotal += n; -} - -Py_ssize_t -_Py_GetRefTotal(void) +static inline Py_ssize_t +get_legacy_reftotal(void) { return _Py_RefTotal; } +#endif + +#ifdef Py_REF_DEBUG + +# define REFTOTAL(runtime) \ + (runtime)->object_state.reftotal + +static inline void +reftotal_increment(_PyRuntimeState *runtime) +{ + REFTOTAL(runtime)++; +} + +static inline void +reftotal_decrement(_PyRuntimeState *runtime) +{ + REFTOTAL(runtime)--; +} + +static inline void +reftotal_add(_PyRuntimeState *runtime, Py_ssize_t n) +{ + REFTOTAL(runtime) += n; +} + +static inline Py_ssize_t get_global_reftotal(_PyRuntimeState *); + +/* We preserve the number of refs leaked during runtime finalization, + so they can be reported if the runtime is initialized again. */ +// XXX We don't lose any information by dropping this, +// so we should consider doing so. +static Py_ssize_t last_final_reftotal = 0; + +void +_Py_FinalizeRefTotal(_PyRuntimeState *runtime) +{ + last_final_reftotal = get_global_reftotal(runtime); + REFTOTAL(runtime) = 0; +} + +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; +} + +#undef REFTOTAL void _PyDebug_PrintTotalRefs(void) { + _PyRuntimeState *runtime = &_PyRuntime; fprintf(stderr, "[%zd refs, %zd blocks]\n", - _Py_GetRefTotal(), _Py_GetAllocatedBlocks()); + get_global_reftotal(runtime), _Py_GetAllocatedBlocks()); + /* It may be helpful to also print the "legacy" reftotal separately. */ } #endif /* Py_REF_DEBUG */ @@ -139,30 +173,50 @@ _Py_NegativeRefcount(const char *filename, int lineno, PyObject *op) filename, lineno, __func__); } -/* This is exposed strictly for use in Py_INCREF(). */ -PyAPI_FUNC(void) +/* This is used strictly by Py_INCREF(). */ +void _Py_IncRefTotal_DO_NOT_USE_THIS(void) { - reftotal_increment(); + reftotal_increment(&_PyRuntime); } -/* This is exposed strictly for use in Py_DECREF(). */ -PyAPI_FUNC(void) +/* This is used strictly by Py_DECREF(). */ +void _Py_DecRefTotal_DO_NOT_USE_THIS(void) { - reftotal_decrement(); + reftotal_decrement(&_PyRuntime); } void _Py_IncRefTotal(void) { - reftotal_increment(); + reftotal_increment(&_PyRuntime); } void _Py_DecRefTotal(void) { - reftotal_decrement(); + reftotal_decrement(&_PyRuntime); +} + +void +_Py_AddRefTotal(Py_ssize_t n) +{ + reftotal_add(&_PyRuntime, n); +} + +/* This includes the legacy total + and any carried over from the last runtime init/fini cycle. */ +Py_ssize_t +_Py_GetGlobalRefTotal(void) +{ + return get_global_reftotal(&_PyRuntime); +} + +Py_ssize_t +_Py_GetLegacyRefTotal(void) +{ + return get_legacy_reftotal(); } #endif /* Py_REF_DEBUG */ @@ -182,21 +236,18 @@ Py_DecRef(PyObject *o) void _Py_IncRef(PyObject *o) { -#ifdef Py_REF_DEBUG - reftotal_increment(); -#endif Py_INCREF(o); } void _Py_DecRef(PyObject *o) { -#ifdef Py_REF_DEBUG - reftotal_decrement(); -#endif Py_DECREF(o); } + +/**************************************/ + PyObject * PyObject_Init(PyObject *op, PyTypeObject *tp) { @@ -2077,7 +2128,7 @@ void _Py_NewReference(PyObject *op) { #ifdef Py_REF_DEBUG - reftotal_increment(); + reftotal_increment(&_PyRuntime); #endif new_reference(op); } diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 0343d30a42c..c754b216574 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -239,6 +239,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 359463e1d9a..90ed0602821 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -621,6 +621,9 @@ Include\internal + + Include\internal + Include\internal diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 317d6966d03..731f340001b 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1930,6 +1930,7 @@ Py_FinalizeEx(void) if (show_ref_count) { _PyDebug_PrintTotalRefs(); } + _Py_FinalizeRefTotal(runtime); #endif #ifdef Py_TRACE_REFS diff --git a/Python/pystate.c b/Python/pystate.c index 3a2966c54a4..4d4213551a8 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -482,6 +482,9 @@ _PyRuntimeState_Init(_PyRuntimeState *runtime) void _PyRuntimeState_Fini(_PyRuntimeState *runtime) { + /* The reftotal is cleared by _Py_FinalizeRefTotal(). */ + assert(runtime->object_state.reftotal == 0); + if (gilstate_tss_initialized(runtime)) { gilstate_tss_fini(runtime); } diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 126b7d422e0..20761738b52 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1854,7 +1854,7 @@ static Py_ssize_t sys_gettotalrefcount_impl(PyObject *module) /*[clinic end generated code: output=4103886cf17c25bc input=53b744faa5d2e4f6]*/ { - return _Py_GetRefTotal(); + return _Py_GetGlobalRefTotal(); } #endif /* Py_REF_DEBUG */ diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 048112dd992..14fc32a07b4 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -141,7 +141,6 @@ Modules/syslogmodule.c - S_log_open - ##----------------------- ## kept for stable ABI compatibility -# XXX should be per-interpreter, without impacting stable ABI extensions Objects/object.c - _Py_RefTotal - ##----------------------- @@ -301,6 +300,7 @@ Objects/genobject.c - NON_INIT_CORO_MSG - Objects/longobject.c - _PyLong_DigitValue - Objects/object.c - _Py_SwappedOp - Objects/object.c - _Py_abstract_hack - +Objects/object.c - last_final_reftotal - Objects/object.c - static_types - Objects/obmalloc.c - _PyMem - Objects/obmalloc.c - _PyMem_Debug -