From 11d48cddbd2501cb918be4aa31bd7889518f4b1d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Aug 2023 10:39:51 -0600 Subject: [PATCH 1/6] Move refchain to _PyRuntimeState. --- Include/internal/pycore_object_state.h | 10 ++++++++-- Include/internal/pycore_runtime_init.h | 11 +++++++++++ Objects/object.c | 7 ++----- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_object_state.h b/Include/internal/pycore_object_state.h index 94005d77881432..35e6ff76b915aa 100644 --- a/Include/internal/pycore_object_state.h +++ b/Include/internal/pycore_object_state.h @@ -11,9 +11,15 @@ extern "C" { struct _py_object_runtime_state { #ifdef Py_REF_DEBUG Py_ssize_t interpreter_leaks; -#else - int _not_used; #endif +#ifdef Py_TRACE_REFS + /* Head of circular doubly-linked list of all objects. These are linked + * together via the _ob_prev and _ob_next members of a PyObject, which + * exist only in a Py_TRACE_REFS build. + */ + PyObject refchain; +#endif + int _not_used; }; struct _py_object_state { diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index dcfceef4f175a2..641cef5b57027d 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -108,6 +108,7 @@ extern PyTypeObject _PyExc_MemoryError; }, \ .faulthandler = _faulthandler_runtime_state_INIT, \ .tracemalloc = _tracemalloc_runtime_state_INIT, \ + .object_state = _py_object_runtime_state_INIT(runtime), \ .float_state = { \ .float_format = _py_float_format_unknown, \ .double_format = _py_float_format_unknown, \ @@ -186,6 +187,16 @@ extern PyTypeObject _PyExc_MemoryError; .context_ver = 1, \ } +#ifdef Py_TRACE_REFS +# define _py_object_runtime_state_INIT(runtime) \ + { \ + .refchain = {&runtime.object_state.refchain, &runtime.object_state.refchain}, \ + } +#else +# define _py_object_runtime_state_INIT(runtime) \ + { 0 } +#endif + // global objects diff --git a/Objects/object.c b/Objects/object.c index 8caa5fd0af3cc9..56ab32d3f8ceaf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -159,11 +159,8 @@ _PyDebug_PrintTotalRefs(void) { Do not call them otherwise, they do not initialize the object! */ #ifdef Py_TRACE_REFS -/* Head of circular doubly-linked list of all objects. These are linked - * together via the _ob_prev and _ob_next members of a PyObject, which - * exist only in a Py_TRACE_REFS build. - */ -static PyObject refchain = {&refchain, &refchain}; + +# define refchain _PyRuntime.object_state.refchain /* Insert op at the front of the list of all objects. If force is true, * op is added even if _ob_prev and _ob_next are non-NULL already. If From 238e00f8338b3dcf29cf2b2d9927474fc35776f5 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Aug 2023 12:09:31 -0600 Subject: [PATCH 2/6] Move refchain to PyInterpreterState. --- Include/internal/pycore_object.h | 5 ++- Include/internal/pycore_object_state.h | 15 +++---- Include/internal/pycore_runtime_init.h | 8 ++-- Objects/object.c | 62 +++++++++++++++++++------- Python/pylifecycle.c | 11 +++-- 5 files changed, 68 insertions(+), 33 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index b730aba9912830..28b1b46a2db5d1 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -292,8 +292,9 @@ extern void _PyDebug_PrintTotalRefs(void); #ifdef Py_TRACE_REFS extern void _Py_AddToAllObjects(PyObject *op, int force); -extern void _Py_PrintReferences(FILE *); -extern void _Py_PrintReferenceAddresses(FILE *); +extern void _Py_PrintReferences(PyInterpreterState *, FILE *); +extern void _Py_PrintReferenceAddresses(PyObject *, FILE *); +extern void _Py_StealRefchain(PyInterpreterState *, PyObject *); #endif diff --git a/Include/internal/pycore_object_state.h b/Include/internal/pycore_object_state.h index 35e6ff76b915aa..65feb5af969f8b 100644 --- a/Include/internal/pycore_object_state.h +++ b/Include/internal/pycore_object_state.h @@ -11,6 +11,13 @@ extern "C" { struct _py_object_runtime_state { #ifdef Py_REF_DEBUG Py_ssize_t interpreter_leaks; +#endif + int _not_used; +}; + +struct _py_object_state { +#ifdef Py_REF_DEBUG + Py_ssize_t reftotal; #endif #ifdef Py_TRACE_REFS /* Head of circular doubly-linked list of all objects. These are linked @@ -22,14 +29,6 @@ struct _py_object_runtime_state { int _not_used; }; -struct _py_object_state { -#ifdef Py_REF_DEBUG - Py_ssize_t reftotal; -#else - int _not_used; -#endif -}; - #ifdef __cplusplus } diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 641cef5b57027d..e89d368be07aa7 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -108,7 +108,6 @@ extern PyTypeObject _PyExc_MemoryError; }, \ .faulthandler = _faulthandler_runtime_state_INIT, \ .tracemalloc = _tracemalloc_runtime_state_INIT, \ - .object_state = _py_object_runtime_state_INIT(runtime), \ .float_state = { \ .float_format = _py_float_format_unknown, \ .double_format = _py_float_format_unknown, \ @@ -158,6 +157,7 @@ extern PyTypeObject _PyExc_MemoryError; { .threshold = 10, }, \ }, \ }, \ + .object_state = _py_object_state_INIT(INTERP), \ .dtoa = _dtoa_state_INIT(&(INTERP)), \ .dict_state = _dict_state_INIT, \ .func_state = { \ @@ -188,12 +188,12 @@ extern PyTypeObject _PyExc_MemoryError; } #ifdef Py_TRACE_REFS -# define _py_object_runtime_state_INIT(runtime) \ +# define _py_object_state_INIT(INTERP) \ { \ - .refchain = {&runtime.object_state.refchain, &runtime.object_state.refchain}, \ + .refchain = {&INTERP.object_state.refchain, &INTERP.object_state.refchain}, \ } #else -# define _py_object_runtime_state_INIT(runtime) \ +# define _py_object_state_INIT(INTERP) \ { 0 } #endif diff --git a/Objects/object.c b/Objects/object.c index 56ab32d3f8ceaf..b029a7049717b2 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -160,7 +160,7 @@ _PyDebug_PrintTotalRefs(void) { #ifdef Py_TRACE_REFS -# define refchain _PyRuntime.object_state.refchain +#define REFCHAIN(interp) &interp->object_state.refchain /* Insert op at the front of the list of all objects. If force is true, * op is added even if _ob_prev and _ob_next are non-NULL already. If @@ -185,10 +185,11 @@ _Py_AddToAllObjects(PyObject *op, int force) } #endif if (force || op->_ob_prev == NULL) { - op->_ob_next = refchain._ob_next; - op->_ob_prev = &refchain; - refchain._ob_next->_ob_prev = op; - refchain._ob_next = op; + PyObject *refchain = REFCHAIN(_PyInterpreterState_GET()); + op->_ob_next = refchain->_ob_next; + op->_ob_prev = refchain; + refchain->_ob_next->_ob_prev = op; + refchain->_ob_next = op; } } #endif /* Py_TRACE_REFS */ @@ -2226,7 +2227,8 @@ _Py_ForgetReference(PyObject *op) _PyObject_ASSERT_FAILED_MSG(op, "negative refcnt"); } - if (op == &refchain || + PyObject *refchain = REFCHAIN(_PyInterpreterState_GET()); + if (op == refchain || op->_ob_prev->_ob_next != op || op->_ob_next->_ob_prev != op) { _PyObject_ASSERT_FAILED_MSG(op, "invalid object chain"); @@ -2234,12 +2236,12 @@ _Py_ForgetReference(PyObject *op) #ifdef SLOW_UNREF_CHECK PyObject *p; - for (p = refchain._ob_next; p != &refchain; p = p->_ob_next) { + for (p = refchain->_ob_next; p != refchain; p = p->_ob_next) { if (p == op) { break; } } - if (p == &refchain) { + if (p == refchain) { /* Not found */ _PyObject_ASSERT_FAILED_MSG(op, "object not found in the objects list"); @@ -2251,15 +2253,41 @@ _Py_ForgetReference(PyObject *op) op->_ob_next = op->_ob_prev = NULL; } +void +_Py_StealRefchain(PyInterpreterState *interp, PyObject *refchain) +{ + assert(refchain != NULL + && (refchain->_ob_prev == NULL || refchain->_ob_prev == refchain) + && (refchain->_ob_next == NULL || refchain->_ob_next == refchain)); + if (interp == NULL) { + interp = _PyInterpreterState_Main(); + } + PyObject *old = REFCHAIN(interp); + if (old->_ob_prev == old) { + assert(old->_ob_next == old); + refchain->_ob_prev = refchain; + refchain->_ob_next = refchain; + return; + } + refchain->_ob_prev = old->_ob_prev; + refchain->_ob_prev->_ob_next = refchain; + refchain->_ob_next = old->_ob_next; + refchain->_ob_next->_ob_prev = refchain; +} + /* Print all live objects. Because PyObject_Print is called, the * interpreter must be in a healthy state. */ void -_Py_PrintReferences(FILE *fp) +_Py_PrintReferences(PyInterpreterState *interp, FILE *fp) { PyObject *op; + if (interp == NULL) { + interp = _PyInterpreterState_Main(); + } fprintf(fp, "Remaining objects:\n"); - for (op = refchain._ob_next; op != &refchain; op = op->_ob_next) { + PyObject *refchain = REFCHAIN(interp); + for (op = refchain->_ob_next; op != refchain; op = op->_ob_next) { fprintf(fp, "%p [%zd] ", (void *)op, Py_REFCNT(op)); if (PyObject_Print(op, fp, 0) != 0) { PyErr_Clear(); @@ -2272,15 +2300,16 @@ _Py_PrintReferences(FILE *fp) * doesn't make any calls to the Python C API, so is always safe to call. */ void -_Py_PrintReferenceAddresses(FILE *fp) +_Py_PrintReferenceAddresses(PyObject *refchain, FILE *fp) { PyObject *op; fprintf(fp, "Remaining object addresses:\n"); - for (op = refchain._ob_next; op != &refchain; op = op->_ob_next) + for (op = refchain->_ob_next; op != refchain; op = op->_ob_next) fprintf(fp, "%p [%zd] %s\n", (void *)op, Py_REFCNT(op), Py_TYPE(op)->tp_name); } +/* The implementation of sys.getobjects(). */ PyObject * _Py_GetObjects(PyObject *self, PyObject *args) { @@ -2290,15 +2319,16 @@ _Py_GetObjects(PyObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "i|O", &n, &t)) return NULL; - op = refchain._ob_next; + PyObject *refchain = REFCHAIN(_PyInterpreterState_GET()); + op = refchain->_ob_next; res = PyList_New(0); if (res == NULL) return NULL; - for (i = 0; (n == 0 || i < n) && op != &refchain; i++) { + for (i = 0; (n == 0 || i < n) && op != refchain; i++) { while (op == self || op == args || op == res || op == t || (t != NULL && !Py_IS_TYPE(op, (PyTypeObject *) t))) { op = op->_ob_next; - if (op == &refchain) + if (op == refchain) return res; } if (PyList_Append(res, op) < 0) { @@ -2310,6 +2340,8 @@ _Py_GetObjects(PyObject *self, PyObject *args) return res; } +#undef REFCHAIN + #endif diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index ceca7776a276ed..305507d183e1a8 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1921,12 +1921,15 @@ Py_FinalizeEx(void) } if (dump_refs) { - _Py_PrintReferences(stderr); + _Py_PrintReferences(tstate->interp, stderr); } if (dump_refs_fp != NULL) { - _Py_PrintReferences(dump_refs_fp); + _Py_PrintReferences(tstate->interp, dump_refs_fp); } + + PyObject refchain = { 0 }; + _Py_StealRefchain(tstate->interp, &refchain); #endif /* Py_TRACE_REFS */ /* At this point there's almost no other Python code that will run, @@ -1961,11 +1964,11 @@ Py_FinalizeEx(void) */ if (dump_refs) { - _Py_PrintReferenceAddresses(stderr); + _Py_PrintReferenceAddresses(&refchain, stderr); } if (dump_refs_fp != NULL) { - _Py_PrintReferenceAddresses(dump_refs_fp); + _Py_PrintReferenceAddresses(&refchain, dump_refs_fp); fclose(dump_refs_fp); } #endif /* Py_TRACE_REFS */ From 33105c0290aaadb11dde7fae19dd19ea48ef3fe2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Aug 2023 12:33:11 -0600 Subject: [PATCH 3/6] Add a NEWS entry. --- .../2023-08-02-12-24-51.gh-issue-107080.PNolFU.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-08-02-12-24-51.gh-issue-107080.PNolFU.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-02-12-24-51.gh-issue-107080.PNolFU.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-02-12-24-51.gh-issue-107080.PNolFU.rst new file mode 100644 index 00000000000000..5084c854360e35 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-02-12-24-51.gh-issue-107080.PNolFU.rst @@ -0,0 +1,4 @@ +Trace refs builds (``--with-trace-refs``) were crashing when used with +isolated subinterpreters. The problematic global state has been isolated to +each interpreter. Other fixing the crashes, this change does not affect +users. From ac78d7bed262b341117a0f0c200a28a6c0527be8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Aug 2023 12:45:19 -0600 Subject: [PATCH 4/6] Clean up _Py_GetObjects(). --- Objects/object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index b029a7049717b2..9c9b3c62866903 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2316,10 +2316,11 @@ _Py_GetObjects(PyObject *self, PyObject *args) int i, n; PyObject *t = NULL; PyObject *res, *op; + PyInterpreterState *interp = _PyInterpreterState_GET(); if (!PyArg_ParseTuple(args, "i|O", &n, &t)) return NULL; - PyObject *refchain = REFCHAIN(_PyInterpreterState_GET()); + PyObject *refchain = REFCHAIN(interp); op = refchain->_ob_next; res = PyList_New(0); if (res == NULL) From e21c558e8dd812e48b34a497a48e527e0272ac25 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 3 Aug 2023 12:40:59 -0600 Subject: [PATCH 5/6] Drop _Py_StealRefchain(). --- Include/internal/pycore_object.h | 3 +-- Objects/object.c | 7 ++++++- Python/pylifecycle.c | 7 ++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 28b1b46a2db5d1..76b3cd69cbf5a7 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -293,8 +293,7 @@ extern void _PyDebug_PrintTotalRefs(void); #ifdef Py_TRACE_REFS extern void _Py_AddToAllObjects(PyObject *op, int force); extern void _Py_PrintReferences(PyInterpreterState *, FILE *); -extern void _Py_PrintReferenceAddresses(PyObject *, FILE *); -extern void _Py_StealRefchain(PyInterpreterState *, PyObject *); +extern void _Py_PrintReferenceAddresses(PyInterpreterState *, FILE *); #endif diff --git a/Objects/object.c b/Objects/object.c index 9c9b3c62866903..055d18306880c2 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2299,10 +2299,15 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) /* Print the addresses of all live objects. Unlike _Py_PrintReferences, this * doesn't make any calls to the Python C API, so is always safe to call. */ +// XXX This function is not safe to use if the interpreter has been +// freed or is in an unhealthy state (e.g. late in finalization). +// The call in Py_FinalizeEx() is okay since the main interpreter +// is statically allocated. void -_Py_PrintReferenceAddresses(PyObject *refchain, FILE *fp) +_Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { PyObject *op; + PyObject *refchain = REFCHAIN(interp); fprintf(fp, "Remaining object addresses:\n"); for (op = refchain->_ob_next; op != refchain; op = op->_ob_next) fprintf(fp, "%p [%zd] %s\n", (void *)op, diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 305507d183e1a8..7a17f92b550c05 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1927,9 +1927,6 @@ Py_FinalizeEx(void) if (dump_refs_fp != NULL) { _Py_PrintReferences(tstate->interp, dump_refs_fp); } - - PyObject refchain = { 0 }; - _Py_StealRefchain(tstate->interp, &refchain); #endif /* Py_TRACE_REFS */ /* At this point there's almost no other Python code that will run, @@ -1964,11 +1961,11 @@ Py_FinalizeEx(void) */ if (dump_refs) { - _Py_PrintReferenceAddresses(&refchain, stderr); + _Py_PrintReferenceAddresses(tstate->interp, stderr); } if (dump_refs_fp != NULL) { - _Py_PrintReferenceAddresses(&refchain, dump_refs_fp); + _Py_PrintReferenceAddresses(tstate->interp, dump_refs_fp); fclose(dump_refs_fp); } #endif /* Py_TRACE_REFS */ From 7a8e5499c91ae8886f86590ae7d2223b193f6ce1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 3 Aug 2023 13:04:59 -0600 Subject: [PATCH 6/6] Drop _Py_StealRefchain(). --- Objects/object.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 055d18306880c2..c4413a00ede80c 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2253,28 +2253,6 @@ _Py_ForgetReference(PyObject *op) op->_ob_next = op->_ob_prev = NULL; } -void -_Py_StealRefchain(PyInterpreterState *interp, PyObject *refchain) -{ - assert(refchain != NULL - && (refchain->_ob_prev == NULL || refchain->_ob_prev == refchain) - && (refchain->_ob_next == NULL || refchain->_ob_next == refchain)); - if (interp == NULL) { - interp = _PyInterpreterState_Main(); - } - PyObject *old = REFCHAIN(interp); - if (old->_ob_prev == old) { - assert(old->_ob_next == old); - refchain->_ob_prev = refchain; - refchain->_ob_next = refchain; - return; - } - refchain->_ob_prev = old->_ob_prev; - refchain->_ob_prev->_ob_next = refchain; - refchain->_ob_next = old->_ob_next; - refchain->_ob_next->_ob_prev = refchain; -} - /* Print all live objects. Because PyObject_Print is called, the * interpreter must be in a healthy state. */