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

diff --git a/Include/cpython/object.h b/Include/cpython/object.h
index 7b687d311359c38ce792b1663f884a47d296344e..0438612edd1dfe89b5c783733590a0ac815ddc9b 100644 (file)
--- 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 318e6f3371c0c35b70167dab56206b6cc7384c5e..b985eff8a8a08b16d0dc96afc3e7ebb0df0fc14d 100644 (file)
--- 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 (file)
index 0000000..4e5862a
--- /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 520109ca440444bf7d35cc8f53fa1c691b192949..de757dfa93bc26e8784974e50a7713d93ba691a2 100644 (file)
--- 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 844b9c4a51c3e47019b14a2ac4402ff2733242e2..fc577353c1cc133f1f455e6d62fc6da155efca5b 100644 (file)
--- 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 8f13198e7e34b321b8f33a78d31db50616ad875c..74e4171b010d0fc917732d82dbfa63c91f44754c 100644 (file)
--- 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 dff5e2afa16ab84c051e0e3e3a0dcb368f79e3f2..95f7c966a414dec192f8a2ee98a4e51f4905ffa1 100644 (file)
--- 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 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_increment(void)
+reftotal_decrement(_PyRuntimeState *runtime)
 {
-    _Py_RefTotal++;
+    REFTOTAL(runtime)--;
 }
 
 static inline void
-reftotal_decrement(void)
+reftotal_add(_PyRuntimeState *runtime, Py_ssize_t n)
 {
-    _Py_RefTotal--;
+    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_AddRefTotal(Py_ssize_t n)
+_Py_FinalizeRefTotal(_PyRuntimeState *runtime)
 {
-    _Py_RefTotal += n;
+    last_final_reftotal = get_global_reftotal(runtime);
+    REFTOTAL(runtime) = 0;
 }
 
-Py_ssize_t
-_Py_GetRefTotal(void)
+static inline Py_ssize_t
+get_global_reftotal(_PyRuntimeState *runtime)
 {
-    return _Py_RefTotal;
+    /* 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 0343d30a42cd6addeba03e3a06ee1c32b72a20cf..c754b2165745ff1996b20788a9c8e22bbea87054 100644 (file)
--- a/PCbuild/pythoncore.vcxproj
+++ b/PCbuild/pythoncore.vcxproj
@@ -239,6 +239,7 @@
     <ClInclude Include="..\Include\internal\pycore_moduleobject.h" />
     <ClInclude Include="..\Include\internal\pycore_namespace.h" />
     <ClInclude Include="..\Include\internal\pycore_object.h" />
+    <ClInclude Include="..\Include\internal\pycore_object_state.h" />
     <ClInclude Include="..\Include\internal\pycore_obmalloc.h" />
     <ClInclude Include="..\Include\internal\pycore_obmalloc_init.h" />
     <ClInclude Include="..\Include\internal\pycore_pathconfig.h" />

diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters
index 359463e1d9af75e54aaddd664b01ca2c185042b1..90ed0602821bfff327c59c60b8a336614f9c6062 100644 (file)
--- a/PCbuild/pythoncore.vcxproj.filters
+++ b/PCbuild/pythoncore.vcxproj.filters
@@ -621,6 +621,9 @@
     <ClInclude Include="..\Include\internal\pycore_object.h">
       <Filter>Include\internal</Filter>
     </ClInclude>
+    <ClInclude Include="..\Include\internal\pycore_object_state.h">
+      <Filter>Include\internal</Filter>
+    </ClInclude>
     <ClInclude Include="..\Include\internal\pycore_obmalloc.h">
       <Filter>Include\internal</Filter>
     </ClInclude>

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 317d6966d034fa961cbcc83649e16f3f1e16f773..731f340001b4e03c569d2dda7a09cd9c1c342d0f 100644 (file)
--- 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 3a2966c54a4c3bb19e45ee011645257654efd50c..4d4213551a8b1cca090f452611135903eb62ab47 100644 (file)
--- 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 126b7d422e00097d3c5b9157f727f0abd6c1ce00..20761738b527cb437dea716cb6c0fc08bc05c3db 100644 (file)
--- 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 048112dd9925557bc26d1b1ee7a27aad3147a822..14fc32a07b4309b7b5c92f0ee1d9eab074eb8a7a 100644 (file)
--- 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    -