From 53cfb82475d1a196347d70f02f266f8efcbc985a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 18 Oct 2024 11:47:09 -0600 Subject: [PATCH 01/16] Share the main refchain with legacy interpreters. --- Objects/object.c | 72 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 57 insertions(+), 15 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 1a15b70d3dc63f..6b017d1086eb8f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -171,9 +171,48 @@ _PyDebug_PrintTotalRefs(void) { #define REFCHAIN(interp) interp->object_state.refchain #define REFCHAIN_VALUE ((void*)(uintptr_t)1) +static int +refchain_init(PyInterpreterState *interp) +{ + PyInterpreterState *main_interp = _PyInterpreterState_Main(); + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC + && interp != main_interp) + { + REFCHAIN(interp) = REFCHAIN(main_interp); + return 0; + } + _Py_hashtable_allocator_t alloc = { + // Don't use default PyMem_Malloc() and PyMem_Free() which + // require the caller to hold the GIL. + .malloc = PyMem_RawMalloc, + .free = PyMem_RawFree, + }; + REFCHAIN(interp) = _Py_hashtable_new_full( + _Py_hashtable_hash_ptr, _Py_hashtable_compare_direct, + NULL, NULL, &alloc); + if (REFCHAIN(interp) == NULL) { + return -1; + } + return 0; +} + +static void +refchain_fini(PyInterpreterState *interp) +{ + if (!(interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) + ||_Py_IsMainInterpreter(interp)) + { + _Py_hashtable_destroy(REFCHAIN(interp)); + } + REFCHAIN(interp) = NULL; +} + bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) { + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } return (_Py_hashtable_get(REFCHAIN(interp), obj) == REFCHAIN_VALUE); } @@ -181,6 +220,9 @@ _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) { + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } if (_Py_hashtable_set(REFCHAIN(interp), obj, REFCHAIN_VALUE) < 0) { // Use a fatal error because _Py_NewReference() cannot report // the error to the caller. @@ -192,6 +234,9 @@ _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Remove(PyInterpreterState *interp, PyObject *obj) { + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } void *value = _Py_hashtable_steal(REFCHAIN(interp), obj); #ifndef NDEBUG assert(value == REFCHAIN_VALUE); @@ -2191,16 +2236,7 @@ PyStatus _PyObject_InitState(PyInterpreterState *interp) { #ifdef Py_TRACE_REFS - _Py_hashtable_allocator_t alloc = { - // Don't use default PyMem_Malloc() and PyMem_Free() which - // require the caller to hold the GIL. - .malloc = PyMem_RawMalloc, - .free = PyMem_RawFree, - }; - REFCHAIN(interp) = _Py_hashtable_new_full( - _Py_hashtable_hash_ptr, _Py_hashtable_compare_direct, - NULL, NULL, &alloc); - if (REFCHAIN(interp) == NULL) { + if (refchain_init(interp) < 0) { return _PyStatus_NO_MEMORY(); } #endif @@ -2211,8 +2247,7 @@ void _PyObject_FiniState(PyInterpreterState *interp) { #ifdef Py_TRACE_REFS - _Py_hashtable_destroy(REFCHAIN(interp)); - REFCHAIN(interp) = NULL; + refchain_fini(interp); #endif } @@ -2531,9 +2566,7 @@ _Py_NormalizeImmortalReference(PyObject *op) if (interp != main_interp && interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { - assert(!_PyRefchain_IsTraced(main_interp, op)); - _PyRefchain_Remove(interp, op); - _PyRefchain_Trace(main_interp, op); + assert(_PyRefchain_IsTraced(main_interp, op)); } } @@ -2583,6 +2616,9 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) interp = _PyInterpreterState_Main(); } fprintf(fp, "Remaining objects:\n"); + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReference, fp); } @@ -2611,6 +2647,9 @@ void _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { fprintf(fp, "Remaining object addresses:\n"); + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReferenceAddress, fp); } @@ -2690,6 +2729,9 @@ _Py_GetObjects(PyObject *self, PyObject *args) .limit = limit, }; PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } int res = _Py_hashtable_foreach(REFCHAIN(interp), _Py_GetObject, &data); if (res == _PY_GETOBJECTS_ERROR) { Py_DECREF(list); From b2bf609eed02093c7f05469e6c23d6dbf4ef9a04 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 18 Oct 2024 15:14:19 -0600 Subject: [PATCH 02/16] Add a has_own_refchain() helper function. --- Objects/object.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 6b017d1086eb8f..6525ae8dae6612 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -171,14 +171,21 @@ _PyDebug_PrintTotalRefs(void) { #define REFCHAIN(interp) interp->object_state.refchain #define REFCHAIN_VALUE ((void*)(uintptr_t)1) +static inline int +has_own_refchain(PyInterpreterState *interp) +{ + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + return _Py_IsMainInterpreter(interp); + } + return 1; +} + static int refchain_init(PyInterpreterState *interp) { - PyInterpreterState *main_interp = _PyInterpreterState_Main(); - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC - && interp != main_interp) - { - REFCHAIN(interp) = REFCHAIN(main_interp); + if (!has_own_refchain(interp)) { + // Legacy subinterpreters share a refchain with the main interpreter. + REFCHAIN(interp) = REFCHAIN(_PyInterpreterState_Main()); return 0; } _Py_hashtable_allocator_t alloc = { @@ -199,9 +206,7 @@ refchain_init(PyInterpreterState *interp) static void refchain_fini(PyInterpreterState *interp) { - if (!(interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) - ||_Py_IsMainInterpreter(interp)) - { + if (has_own_refchain(interp)) { _Py_hashtable_destroy(REFCHAIN(interp)); } REFCHAIN(interp) = NULL; @@ -210,7 +215,7 @@ refchain_fini(PyInterpreterState *interp) bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) { - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_refchain(interp)) { interp = _PyInterpreterState_Main(); } return (_Py_hashtable_get(REFCHAIN(interp), obj) == REFCHAIN_VALUE); @@ -220,7 +225,7 @@ _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) { - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_refchain(interp)) { interp = _PyInterpreterState_Main(); } if (_Py_hashtable_set(REFCHAIN(interp), obj, REFCHAIN_VALUE) < 0) { @@ -234,7 +239,7 @@ _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Remove(PyInterpreterState *interp, PyObject *obj) { - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_refchain(interp)) { interp = _PyInterpreterState_Main(); } void *value = _Py_hashtable_steal(REFCHAIN(interp), obj); @@ -2562,12 +2567,10 @@ _Py_NormalizeImmortalReference(PyObject *op) if (!_PyRefchain_IsTraced(interp, op)) { return; } - PyInterpreterState *main_interp = _PyInterpreterState_Main(); - if (interp != main_interp - && interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) - { - assert(_PyRefchain_IsTraced(main_interp, op)); + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); } + assert(_PyRefchain_IsTraced(interp, op)); } void @@ -2616,7 +2619,7 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) interp = _PyInterpreterState_Main(); } fprintf(fp, "Remaining objects:\n"); - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_refchain(interp)) { interp = _PyInterpreterState_Main(); } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReference, fp); @@ -2647,7 +2650,7 @@ void _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { fprintf(fp, "Remaining object addresses:\n"); - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_refchain(interp)) { interp = _PyInterpreterState_Main(); } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReferenceAddress, fp); @@ -2729,7 +2732,7 @@ _Py_GetObjects(PyObject *self, PyObject *args) .limit = limit, }; PyInterpreterState *interp = _PyInterpreterState_GET(); - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_refchain(interp)) { interp = _PyInterpreterState_Main(); } int res = _Py_hashtable_foreach(REFCHAIN(interp), _Py_GetObject, &data); From 203c112b37fa554359590e51bc3c423eedf459c4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 14:29:18 -0600 Subject: [PATCH 03/16] Drop _Py_NormalizeImmortalReference(). --- Objects/object.c | 32 -------------------------------- Objects/unicodeobject.c | 8 -------- 2 files changed, 40 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 6525ae8dae6612..745246b727f757 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2541,38 +2541,6 @@ _Py_ResurrectReference(PyObject *op) #ifdef Py_TRACE_REFS -/* Make sure the ref is associated with the right interpreter. - * This only needs special attention for heap-allocated objects - * that have been immortalized, and only when the object might - * outlive the interpreter where it was created. That means the - * object was necessarily created using a global allocator - * (i.e. from the main interpreter). Thus in that specific case - * we move the object over to the main interpreter's refchain. - * - * This was added for the sake of the immortal interned strings, - * where legacy subinterpreters share the main interpreter's - * interned dict (and allocator), and therefore the strings can - * outlive the subinterpreter. - * - * It may make sense to fold this into _Py_SetImmortalUntracked(), - * but that requires further investigation. In the meantime, it is - * up to the caller to know if this is needed. There should be - * very few cases. - */ -void -_Py_NormalizeImmortalReference(PyObject *op) -{ - assert(_Py_IsImmortal(op)); - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!_PyRefchain_IsTraced(interp, op)) { - return; - } - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } - assert(_PyRefchain_IsTraced(interp, op)); -} - void _Py_ForgetReference(PyObject *op) { diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index b94a74c2c688a9..9cd9781e412524 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15444,10 +15444,6 @@ _PyUnicode_InternStatic(PyInterpreterState *interp, PyObject **p) assert(*p); } -#ifdef Py_TRACE_REFS -extern void _Py_NormalizeImmortalReference(PyObject *); -#endif - static void immortalize_interned(PyObject *s) { @@ -15463,10 +15459,6 @@ immortalize_interned(PyObject *s) #endif _PyUnicode_STATE(s).interned = SSTATE_INTERNED_IMMORTAL; _Py_SetImmortal(s); -#ifdef Py_TRACE_REFS - /* Make sure the ref is associated with the right interpreter. */ - _Py_NormalizeImmortalReference(s); -#endif } static /* non-null */ PyObject* From ffe263370fe0ac0f2c5bde12821775b8943eb24f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 14:26:19 -0600 Subject: [PATCH 04/16] Drop has_own_refchain() calls where not needed. --- Objects/object.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 745246b727f757..aaff05ec355ee6 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -215,9 +215,6 @@ refchain_fini(PyInterpreterState *interp) bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) { - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } return (_Py_hashtable_get(REFCHAIN(interp), obj) == REFCHAIN_VALUE); } @@ -225,9 +222,6 @@ _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) { - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } if (_Py_hashtable_set(REFCHAIN(interp), obj, REFCHAIN_VALUE) < 0) { // Use a fatal error because _Py_NewReference() cannot report // the error to the caller. @@ -239,9 +233,6 @@ _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Remove(PyInterpreterState *interp, PyObject *obj) { - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } void *value = _Py_hashtable_steal(REFCHAIN(interp), obj); #ifndef NDEBUG assert(value == REFCHAIN_VALUE); @@ -2587,9 +2578,6 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) interp = _PyInterpreterState_Main(); } fprintf(fp, "Remaining objects:\n"); - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReference, fp); } @@ -2618,9 +2606,6 @@ void _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { fprintf(fp, "Remaining object addresses:\n"); - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReferenceAddress, fp); } @@ -2700,9 +2685,6 @@ _Py_GetObjects(PyObject *self, PyObject *args) .limit = limit, }; PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } int res = _Py_hashtable_foreach(REFCHAIN(interp), _Py_GetObject, &data); if (res == _PY_GETOBJECTS_ERROR) { Py_DECREF(list); From 8d522588c46fdd55ecb88b04fd92edc5ebd0ea14 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 15:36:01 -0600 Subject: [PATCH 05/16] Call _PyObject_InitState() *after* computing the feature flags. --- Python/pylifecycle.c | 10 ++++++++++ Python/pystate.c | 7 +++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index b8f424854ecb86..b95b9118e104bc 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -674,6 +674,16 @@ pycore_create_interpreter(_PyRuntimeState *runtime, return status; } + // Ideally we would call this in _PyInterpreterState_New(), but it + // requires interp->feature_flags to be correctly set already. + // That mwans we'd have to compute the feature flags before + // creating the interpreter and passing them in there, which we + // should probably be doing anyway. + status = _PyObject_InitState(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. diff --git a/Python/pystate.c b/Python/pystate.c index 7df872cd6d7d8a..5fe0016d4aa56b 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -629,10 +629,9 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; - PyStatus status = _PyObject_InitState(interp); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + // We would call _PyObject_InitState() here, but it needs the + // interp->feature_flags to be correctly set already, which we don't + // currently do. _PyEval_InitState(interp); _PyGC_InitState(&interp->gc); From 216dde2c3b1a0b57033e62837a92a333fa957fd3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 15:41:51 -0600 Subject: [PATCH 06/16] Revert "Call _PyObject_InitState() *after* computing the feature flags." This reverts commit 8d522588c46fdd55ecb88b04fd92edc5ebd0ea14. --- Python/pylifecycle.c | 10 ---------- Python/pystate.c | 7 ++++--- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index b95b9118e104bc..b8f424854ecb86 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -674,16 +674,6 @@ pycore_create_interpreter(_PyRuntimeState *runtime, return status; } - // Ideally we would call this in _PyInterpreterState_New(), but it - // requires interp->feature_flags to be correctly set already. - // That mwans we'd have to compute the feature flags before - // creating the interpreter and passing them in there, which we - // should probably be doing anyway. - status = _PyObject_InitState(interp); - if (_PyStatus_EXCEPTION(status)) { - return status; - } - // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. diff --git a/Python/pystate.c b/Python/pystate.c index 5fe0016d4aa56b..7df872cd6d7d8a 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -629,9 +629,10 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; - // We would call _PyObject_InitState() here, but it needs the - // interp->feature_flags to be correctly set already, which we don't - // currently do. + PyStatus status = _PyObject_InitState(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } _PyEval_InitState(interp); _PyGC_InitState(&interp->gc); From fbc4e2dac0eb27adac50cee1c7fb60ab33fc0817 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 15:55:16 -0600 Subject: [PATCH 07/16] Revert "Drop has_own_refchain() calls where not needed." This reverts commit ffe263370fe0ac0f2c5bde12821775b8943eb24f. --- Objects/object.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Objects/object.c b/Objects/object.c index aaff05ec355ee6..745246b727f757 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -215,6 +215,9 @@ refchain_fini(PyInterpreterState *interp) bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) { + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); + } return (_Py_hashtable_get(REFCHAIN(interp), obj) == REFCHAIN_VALUE); } @@ -222,6 +225,9 @@ _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) { + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); + } if (_Py_hashtable_set(REFCHAIN(interp), obj, REFCHAIN_VALUE) < 0) { // Use a fatal error because _Py_NewReference() cannot report // the error to the caller. @@ -233,6 +239,9 @@ _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Remove(PyInterpreterState *interp, PyObject *obj) { + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); + } void *value = _Py_hashtable_steal(REFCHAIN(interp), obj); #ifndef NDEBUG assert(value == REFCHAIN_VALUE); @@ -2578,6 +2587,9 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) interp = _PyInterpreterState_Main(); } fprintf(fp, "Remaining objects:\n"); + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); + } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReference, fp); } @@ -2606,6 +2618,9 @@ void _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { fprintf(fp, "Remaining object addresses:\n"); + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); + } _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReferenceAddress, fp); } @@ -2685,6 +2700,9 @@ _Py_GetObjects(PyObject *self, PyObject *args) .limit = limit, }; PyInterpreterState *interp = _PyInterpreterState_GET(); + if (!has_own_refchain(interp)) { + interp = _PyInterpreterState_Main(); + } int res = _Py_hashtable_foreach(REFCHAIN(interp), _Py_GetObject, &data); if (res == _PY_GETOBJECTS_ERROR) { Py_DECREF(list); From cb0684719ae0efbfb6969ded9ca03f2ce372c430 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 17:02:03 -0600 Subject: [PATCH 08/16] has_own_refchain() -> maybe_fix_refchain() --- Objects/object.c | 72 ++++++++++++++++++++++++++++++++++++------------ Python/pystate.c | 3 ++ 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 745246b727f757..3a4d0972cbf65e 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -209,15 +209,61 @@ refchain_fini(PyInterpreterState *interp) if (has_own_refchain(interp)) { _Py_hashtable_destroy(REFCHAIN(interp)); } + // This extra branch can be removed once we start setting + // interp->feature_flags *before* refchain_init() gets called. + else if (REFCHAIN(interp) != REFCHAIN(_PyInterpreterState_Main())) { + _Py_hashtable_destroy(REFCHAIN(interp)); + } REFCHAIN(interp) = NULL; } +/* Currently refchain_init() is called during interpreter initialization, + via _PyObject_InitState(), before interp->feature_flags is set, + so an interpreter may have its own refchain even though it shouldn't. + Until that gets fixed, we patch it all up wherever apprpriate. + maybe_fix_refchain() and move_refchain_item() can be dropped + once we sort out the init order problem. */ + +static int +copy_refchain_item(_Py_hashtable_t *ht, + const void *key, const void *value, + void *user_data) +{ + if (value != REFCHAIN_VALUE) { + assert(value == NULL); + return 0; + } + _Py_hashtable_t *new_chain = (_Py_hashtable_t *)user_data; + if (_Py_hashtable_set(new_chain, key, REFCHAIN_VALUE) < 0) { + Py_FatalError("_Py_hashtable_set() memory allocation failed"); + } + return 0; +} + +static void +maybe_fix_refchain(PyInterpreterState *interp) +{ + if (has_own_refchain(interp)) { + // It's okay or we haven't set the feature flags correctly yet. + return; + } + + _Py_hashtable_t *cur_chain = REFCHAIN(interp); + _Py_hashtable_t *main_chain = REFCHAIN(_PyInterpreterState_Main()); + if (cur_chain == main_chain) { + // It was already fixed. + return; + } + REFCHAIN(interp) = main_chain; + + (void)_Py_hashtable_foreach(cur_chain, copy_refchain_item, main_chain); + _Py_hashtable_destroy(cur_chain); +} + bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) { - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } + maybe_fix_refchain(interp); return (_Py_hashtable_get(REFCHAIN(interp), obj) == REFCHAIN_VALUE); } @@ -225,9 +271,7 @@ _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) { - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } + maybe_fix_refchain(interp); if (_Py_hashtable_set(REFCHAIN(interp), obj, REFCHAIN_VALUE) < 0) { // Use a fatal error because _Py_NewReference() cannot report // the error to the caller. @@ -239,9 +283,7 @@ _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Remove(PyInterpreterState *interp, PyObject *obj) { - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } + maybe_fix_refchain(interp); void *value = _Py_hashtable_steal(REFCHAIN(interp), obj); #ifndef NDEBUG assert(value == REFCHAIN_VALUE); @@ -2587,9 +2629,7 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) interp = _PyInterpreterState_Main(); } fprintf(fp, "Remaining objects:\n"); - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } + maybe_fix_refchain(interp); _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReference, fp); } @@ -2618,9 +2658,7 @@ void _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { fprintf(fp, "Remaining object addresses:\n"); - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } + maybe_fix_refchain(interp); _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReferenceAddress, fp); } @@ -2700,9 +2738,7 @@ _Py_GetObjects(PyObject *self, PyObject *args) .limit = limit, }; PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!has_own_refchain(interp)) { - interp = _PyInterpreterState_Main(); - } + maybe_fix_refchain(interp); int res = _Py_hashtable_foreach(REFCHAIN(interp), _Py_GetObject, &data); if (res == _PY_GETOBJECTS_ERROR) { Py_DECREF(list); diff --git a/Python/pystate.c b/Python/pystate.c index 7df872cd6d7d8a..7f734c4501ff9a 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -629,6 +629,9 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; + // This relies on interp->feature_flags being set already, + // but currently we don't set that by this point. + // That's something to fix. PyStatus status = _PyObject_InitState(interp); if (_PyStatus_EXCEPTION(status)) { return status; From 7aa33a9e2bc448d403eac31be8fdb3fa647116d1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 17:02:26 -0600 Subject: [PATCH 09/16] Handle late runtime fini correctly. --- Objects/object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index 3a4d0972cbf65e..16da48edee2663 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -175,7 +175,8 @@ static inline int has_own_refchain(PyInterpreterState *interp) { if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { - return _Py_IsMainInterpreter(interp); + return (_Py_IsMainInterpreter(interp) + || _PyInterpreterState_Main() == NULL); } return 1; } From 3421a1996960c1148fc72954fb46e09d43be2e75 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 18:06:15 -0600 Subject: [PATCH 10/16] Pass the interpreter config to init_interpreter(). --- Include/internal/pycore_interp.h | 1 + Python/pylifecycle.c | 96 ++++++-------------------------- Python/pystate.c | 77 +++++++++++++++++++++++-- 3 files changed, 89 insertions(+), 85 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 36cd71e5a007d5..ac27e7391e8734 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -399,6 +399,7 @@ extern int _PyInterpreterState_HasFeature(PyInterpreterState *interp, PyAPI_FUNC(PyStatus) _PyInterpreterState_New( PyThreadState *tstate, + const PyInterpreterConfig *config, PyInterpreterState **pinterp); diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index b8f424854ecb86..0f098d4b1635df 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -551,62 +551,6 @@ pycore_init_runtime(_PyRuntimeState *runtime, } -static PyStatus -init_interp_settings(PyInterpreterState *interp, - const PyInterpreterConfig *config) -{ - assert(interp->feature_flags == 0); - - if (config->use_main_obmalloc) { - interp->feature_flags |= Py_RTFLAGS_USE_MAIN_OBMALLOC; - } - else if (!config->check_multi_interp_extensions) { - /* The reason: PyModuleDef.m_base.m_copy leaks objects between - interpreters. */ - return _PyStatus_ERR("per-interpreter obmalloc does not support " - "single-phase init extension modules"); - } -#ifdef Py_GIL_DISABLED - if (!_Py_IsMainInterpreter(interp) && - !config->check_multi_interp_extensions) - { - return _PyStatus_ERR("The free-threaded build does not support " - "single-phase init extension modules in " - "subinterpreters"); - } -#endif - - if (config->allow_fork) { - interp->feature_flags |= Py_RTFLAGS_FORK; - } - if (config->allow_exec) { - interp->feature_flags |= Py_RTFLAGS_EXEC; - } - // Note that fork+exec is always allowed. - - if (config->allow_threads) { - interp->feature_flags |= Py_RTFLAGS_THREADS; - } - if (config->allow_daemon_threads) { - interp->feature_flags |= Py_RTFLAGS_DAEMON_THREADS; - } - - if (config->check_multi_interp_extensions) { - interp->feature_flags |= Py_RTFLAGS_MULTI_INTERP_EXTENSIONS; - } - - switch (config->gil) { - case PyInterpreterConfig_DEFAULT_GIL: break; - case PyInterpreterConfig_SHARED_GIL: break; - case PyInterpreterConfig_OWN_GIL: break; - default: - return _PyStatus_ERR("invalid interpreter config 'gil' value"); - } - - return _PyStatus_OK(); -} - - static void init_interp_create_gil(PyThreadState *tstate, int gil) { @@ -643,8 +587,15 @@ pycore_create_interpreter(_PyRuntimeState *runtime, PyThreadState **tstate_p) { PyStatus status; + + PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; + // The main interpreter always has its own GIL and supports single-phase + // init extensions. + config.gil = PyInterpreterConfig_OWN_GIL; + config.check_multi_interp_extensions = 0; + PyInterpreterState *interp; - status = _PyInterpreterState_New(NULL, &interp); + status = _PyInterpreterState_New(NULL, &config, &interp); if (_PyStatus_EXCEPTION(status)) { return status; } @@ -664,16 +615,6 @@ pycore_create_interpreter(_PyRuntimeState *runtime, return status; } - PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; - // The main interpreter always has its own GIL and supports single-phase - // init extensions. - config.gil = PyInterpreterConfig_OWN_GIL; - config.check_multi_interp_extensions = 0; - status = init_interp_settings(interp, &config); - if (_PyStatus_EXCEPTION(status)) { - return status; - } - // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. @@ -2253,18 +2194,19 @@ new_interpreter(PyThreadState **tstate_p, interpreters: disable PyGILState_Check(). */ runtime->gilstate.check_enabled = 0; - PyInterpreterState *interp = PyInterpreterState_New(); - if (interp == NULL) { - *tstate_p = NULL; - return _PyStatus_OK(); - } - _PyInterpreterState_SetWhence(interp, whence); - interp->_ready = 1; // XXX Might new_interpreter() have been called without the GIL held? PyThreadState *save_tstate = _PyThreadState_GET(); PyThreadState *tstate = NULL; + PyInterpreterState *interp; + status = _PyInterpreterState_New(save_tstate, config, &interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + _PyInterpreterState_SetWhence(interp, whence); + interp->_ready = 1; + /* From this point until the init_interp_create_gil() call, we must not do anything that requires that the GIL be held (or otherwise exist). That applies whether or not the new @@ -2291,12 +2233,6 @@ new_interpreter(PyThreadState **tstate_p, goto error; } - /* This does not require that the GIL be held. */ - status = init_interp_settings(interp, config); - if (_PyStatus_EXCEPTION(status)) { - goto error; - } - // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. diff --git a/Python/pystate.c b/Python/pystate.c index 7f734c4501ff9a..10ac15c59db7c4 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -581,9 +581,66 @@ free_interpreter(PyInterpreterState *interp) PyMem_RawFree(interp); } } + +static PyStatus +init_interp_settings(PyInterpreterState *interp, + const PyInterpreterConfig *config) +{ + assert(interp->feature_flags == 0); + + if (config->use_main_obmalloc) { + interp->feature_flags |= Py_RTFLAGS_USE_MAIN_OBMALLOC; + } + else if (!config->check_multi_interp_extensions) { + /* The reason: PyModuleDef.m_base.m_copy leaks objects between + interpreters. */ + return _PyStatus_ERR("per-interpreter obmalloc does not support " + "single-phase init extension modules"); + } +#ifdef Py_GIL_DISABLED + if (!_Py_IsMainInterpreter(interp) && + !config->check_multi_interp_extensions) + { + return _PyStatus_ERR("The free-threaded build does not support " + "single-phase init extension modules in " + "subinterpreters"); + } +#endif + + if (config->allow_fork) { + interp->feature_flags |= Py_RTFLAGS_FORK; + } + if (config->allow_exec) { + interp->feature_flags |= Py_RTFLAGS_EXEC; + } + // Note that fork+exec is always allowed. + + if (config->allow_threads) { + interp->feature_flags |= Py_RTFLAGS_THREADS; + } + if (config->allow_daemon_threads) { + interp->feature_flags |= Py_RTFLAGS_DAEMON_THREADS; + } + + if (config->check_multi_interp_extensions) { + interp->feature_flags |= Py_RTFLAGS_MULTI_INTERP_EXTENSIONS; + } + + switch (config->gil) { + case PyInterpreterConfig_DEFAULT_GIL: break; + case PyInterpreterConfig_SHARED_GIL: break; + case PyInterpreterConfig_OWN_GIL: break; + default: + return _PyStatus_ERR("invalid interpreter config 'gil' value"); + } + + return _PyStatus_OK(); +} + #ifndef NDEBUG static inline int check_interpreter_whence(long); #endif + /* Get the interpreter state to a minimal consistent state. Further init happens in pylifecycle.c before it can be used. All fields not initialized here are expected to be zeroed out, @@ -607,7 +664,8 @@ static PyStatus init_interpreter(PyInterpreterState *interp, _PyRuntimeState *runtime, int64_t id, PyInterpreterState *next, - long whence) + long whence, + const PyInterpreterConfig *config) { if (interp->_initialized) { return _PyStatus_ERR("interpreter already initialized"); @@ -629,10 +687,15 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; + PyStatus status = init_interp_settings(interp, config); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + // This relies on interp->feature_flags being set already, // but currently we don't set that by this point. // That's something to fix. - PyStatus status = _PyObject_InitState(interp); + status = _PyObject_InitState(interp); if (_PyStatus_EXCEPTION(status)) { return status; } @@ -673,7 +736,9 @@ init_interpreter(PyInterpreterState *interp, PyStatus -_PyInterpreterState_New(PyThreadState *tstate, PyInterpreterState **pinterp) +_PyInterpreterState_New(PyThreadState *tstate, + const PyInterpreterConfig *config, + PyInterpreterState **pinterp) { *pinterp = NULL; @@ -735,7 +800,7 @@ _PyInterpreterState_New(PyThreadState *tstate, PyInterpreterState **pinterp) long whence = _PyInterpreterState_WHENCE_UNKNOWN; status = init_interpreter(interp, runtime, - id, old_head, whence); + id, old_head, whence, config); if (_PyStatus_EXCEPTION(status)) { goto error; } @@ -762,8 +827,10 @@ PyInterpreterState_New(void) // tstate can be NULL PyThreadState *tstate = current_fast_get(); + const PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; + PyInterpreterState *interp; - PyStatus status = _PyInterpreterState_New(tstate, &interp); + PyStatus status = _PyInterpreterState_New(tstate, &config, &interp); if (_PyStatus_EXCEPTION(status)) { Py_ExitStatusException(status); } From b276454b1e92ca3f2917d06f17a461c59df243f7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 18:07:24 -0600 Subject: [PATCH 11/16] Revert "Pass the interpreter config to init_interpreter()." This reverts commit 3421a1996960c1148fc72954fb46e09d43be2e75. --- Include/internal/pycore_interp.h | 1 - Python/pylifecycle.c | 96 ++++++++++++++++++++++++++------ Python/pystate.c | 77 ++----------------------- 3 files changed, 85 insertions(+), 89 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index ac27e7391e8734..36cd71e5a007d5 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -399,7 +399,6 @@ extern int _PyInterpreterState_HasFeature(PyInterpreterState *interp, PyAPI_FUNC(PyStatus) _PyInterpreterState_New( PyThreadState *tstate, - const PyInterpreterConfig *config, PyInterpreterState **pinterp); diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 0f098d4b1635df..b8f424854ecb86 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -551,6 +551,62 @@ pycore_init_runtime(_PyRuntimeState *runtime, } +static PyStatus +init_interp_settings(PyInterpreterState *interp, + const PyInterpreterConfig *config) +{ + assert(interp->feature_flags == 0); + + if (config->use_main_obmalloc) { + interp->feature_flags |= Py_RTFLAGS_USE_MAIN_OBMALLOC; + } + else if (!config->check_multi_interp_extensions) { + /* The reason: PyModuleDef.m_base.m_copy leaks objects between + interpreters. */ + return _PyStatus_ERR("per-interpreter obmalloc does not support " + "single-phase init extension modules"); + } +#ifdef Py_GIL_DISABLED + if (!_Py_IsMainInterpreter(interp) && + !config->check_multi_interp_extensions) + { + return _PyStatus_ERR("The free-threaded build does not support " + "single-phase init extension modules in " + "subinterpreters"); + } +#endif + + if (config->allow_fork) { + interp->feature_flags |= Py_RTFLAGS_FORK; + } + if (config->allow_exec) { + interp->feature_flags |= Py_RTFLAGS_EXEC; + } + // Note that fork+exec is always allowed. + + if (config->allow_threads) { + interp->feature_flags |= Py_RTFLAGS_THREADS; + } + if (config->allow_daemon_threads) { + interp->feature_flags |= Py_RTFLAGS_DAEMON_THREADS; + } + + if (config->check_multi_interp_extensions) { + interp->feature_flags |= Py_RTFLAGS_MULTI_INTERP_EXTENSIONS; + } + + switch (config->gil) { + case PyInterpreterConfig_DEFAULT_GIL: break; + case PyInterpreterConfig_SHARED_GIL: break; + case PyInterpreterConfig_OWN_GIL: break; + default: + return _PyStatus_ERR("invalid interpreter config 'gil' value"); + } + + return _PyStatus_OK(); +} + + static void init_interp_create_gil(PyThreadState *tstate, int gil) { @@ -587,15 +643,8 @@ pycore_create_interpreter(_PyRuntimeState *runtime, PyThreadState **tstate_p) { PyStatus status; - - PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; - // The main interpreter always has its own GIL and supports single-phase - // init extensions. - config.gil = PyInterpreterConfig_OWN_GIL; - config.check_multi_interp_extensions = 0; - PyInterpreterState *interp; - status = _PyInterpreterState_New(NULL, &config, &interp); + status = _PyInterpreterState_New(NULL, &interp); if (_PyStatus_EXCEPTION(status)) { return status; } @@ -615,6 +664,16 @@ pycore_create_interpreter(_PyRuntimeState *runtime, return status; } + PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; + // The main interpreter always has its own GIL and supports single-phase + // init extensions. + config.gil = PyInterpreterConfig_OWN_GIL; + config.check_multi_interp_extensions = 0; + status = init_interp_settings(interp, &config); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. @@ -2194,19 +2253,18 @@ new_interpreter(PyThreadState **tstate_p, interpreters: disable PyGILState_Check(). */ runtime->gilstate.check_enabled = 0; + PyInterpreterState *interp = PyInterpreterState_New(); + if (interp == NULL) { + *tstate_p = NULL; + return _PyStatus_OK(); + } + _PyInterpreterState_SetWhence(interp, whence); + interp->_ready = 1; // XXX Might new_interpreter() have been called without the GIL held? PyThreadState *save_tstate = _PyThreadState_GET(); PyThreadState *tstate = NULL; - PyInterpreterState *interp; - status = _PyInterpreterState_New(save_tstate, config, &interp); - if (_PyStatus_EXCEPTION(status)) { - return status; - } - _PyInterpreterState_SetWhence(interp, whence); - interp->_ready = 1; - /* From this point until the init_interp_create_gil() call, we must not do anything that requires that the GIL be held (or otherwise exist). That applies whether or not the new @@ -2233,6 +2291,12 @@ new_interpreter(PyThreadState **tstate_p, goto error; } + /* This does not require that the GIL be held. */ + status = init_interp_settings(interp, config); + if (_PyStatus_EXCEPTION(status)) { + goto error; + } + // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. diff --git a/Python/pystate.c b/Python/pystate.c index 10ac15c59db7c4..7f734c4501ff9a 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -581,66 +581,9 @@ free_interpreter(PyInterpreterState *interp) PyMem_RawFree(interp); } } - -static PyStatus -init_interp_settings(PyInterpreterState *interp, - const PyInterpreterConfig *config) -{ - assert(interp->feature_flags == 0); - - if (config->use_main_obmalloc) { - interp->feature_flags |= Py_RTFLAGS_USE_MAIN_OBMALLOC; - } - else if (!config->check_multi_interp_extensions) { - /* The reason: PyModuleDef.m_base.m_copy leaks objects between - interpreters. */ - return _PyStatus_ERR("per-interpreter obmalloc does not support " - "single-phase init extension modules"); - } -#ifdef Py_GIL_DISABLED - if (!_Py_IsMainInterpreter(interp) && - !config->check_multi_interp_extensions) - { - return _PyStatus_ERR("The free-threaded build does not support " - "single-phase init extension modules in " - "subinterpreters"); - } -#endif - - if (config->allow_fork) { - interp->feature_flags |= Py_RTFLAGS_FORK; - } - if (config->allow_exec) { - interp->feature_flags |= Py_RTFLAGS_EXEC; - } - // Note that fork+exec is always allowed. - - if (config->allow_threads) { - interp->feature_flags |= Py_RTFLAGS_THREADS; - } - if (config->allow_daemon_threads) { - interp->feature_flags |= Py_RTFLAGS_DAEMON_THREADS; - } - - if (config->check_multi_interp_extensions) { - interp->feature_flags |= Py_RTFLAGS_MULTI_INTERP_EXTENSIONS; - } - - switch (config->gil) { - case PyInterpreterConfig_DEFAULT_GIL: break; - case PyInterpreterConfig_SHARED_GIL: break; - case PyInterpreterConfig_OWN_GIL: break; - default: - return _PyStatus_ERR("invalid interpreter config 'gil' value"); - } - - return _PyStatus_OK(); -} - #ifndef NDEBUG static inline int check_interpreter_whence(long); #endif - /* Get the interpreter state to a minimal consistent state. Further init happens in pylifecycle.c before it can be used. All fields not initialized here are expected to be zeroed out, @@ -664,8 +607,7 @@ static PyStatus init_interpreter(PyInterpreterState *interp, _PyRuntimeState *runtime, int64_t id, PyInterpreterState *next, - long whence, - const PyInterpreterConfig *config) + long whence) { if (interp->_initialized) { return _PyStatus_ERR("interpreter already initialized"); @@ -687,15 +629,10 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; - PyStatus status = init_interp_settings(interp, config); - if (_PyStatus_EXCEPTION(status)) { - return status; - } - // This relies on interp->feature_flags being set already, // but currently we don't set that by this point. // That's something to fix. - status = _PyObject_InitState(interp); + PyStatus status = _PyObject_InitState(interp); if (_PyStatus_EXCEPTION(status)) { return status; } @@ -736,9 +673,7 @@ init_interpreter(PyInterpreterState *interp, PyStatus -_PyInterpreterState_New(PyThreadState *tstate, - const PyInterpreterConfig *config, - PyInterpreterState **pinterp) +_PyInterpreterState_New(PyThreadState *tstate, PyInterpreterState **pinterp) { *pinterp = NULL; @@ -800,7 +735,7 @@ _PyInterpreterState_New(PyThreadState *tstate, long whence = _PyInterpreterState_WHENCE_UNKNOWN; status = init_interpreter(interp, runtime, - id, old_head, whence, config); + id, old_head, whence); if (_PyStatus_EXCEPTION(status)) { goto error; } @@ -827,10 +762,8 @@ PyInterpreterState_New(void) // tstate can be NULL PyThreadState *tstate = current_fast_get(); - const PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; - PyInterpreterState *interp; - PyStatus status = _PyInterpreterState_New(tstate, &config, &interp); + PyStatus status = _PyInterpreterState_New(tstate, &interp); if (_PyStatus_EXCEPTION(status)) { Py_ExitStatusException(status); } From 7ab086c86123a8556317afd390bf416b07615da6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2024 18:15:07 -0600 Subject: [PATCH 12/16] Move the _PyObject_InitState() call out of init_interpreter(). --- Objects/object.c | 54 -------------------------------------------- Python/pylifecycle.c | 14 ++++++++++++ Python/pystate.c | 9 ++------ 3 files changed, 16 insertions(+), 61 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 16da48edee2663..a186f7da24e92c 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -210,61 +210,12 @@ refchain_fini(PyInterpreterState *interp) if (has_own_refchain(interp)) { _Py_hashtable_destroy(REFCHAIN(interp)); } - // This extra branch can be removed once we start setting - // interp->feature_flags *before* refchain_init() gets called. - else if (REFCHAIN(interp) != REFCHAIN(_PyInterpreterState_Main())) { - _Py_hashtable_destroy(REFCHAIN(interp)); - } REFCHAIN(interp) = NULL; } -/* Currently refchain_init() is called during interpreter initialization, - via _PyObject_InitState(), before interp->feature_flags is set, - so an interpreter may have its own refchain even though it shouldn't. - Until that gets fixed, we patch it all up wherever apprpriate. - maybe_fix_refchain() and move_refchain_item() can be dropped - once we sort out the init order problem. */ - -static int -copy_refchain_item(_Py_hashtable_t *ht, - const void *key, const void *value, - void *user_data) -{ - if (value != REFCHAIN_VALUE) { - assert(value == NULL); - return 0; - } - _Py_hashtable_t *new_chain = (_Py_hashtable_t *)user_data; - if (_Py_hashtable_set(new_chain, key, REFCHAIN_VALUE) < 0) { - Py_FatalError("_Py_hashtable_set() memory allocation failed"); - } - return 0; -} - -static void -maybe_fix_refchain(PyInterpreterState *interp) -{ - if (has_own_refchain(interp)) { - // It's okay or we haven't set the feature flags correctly yet. - return; - } - - _Py_hashtable_t *cur_chain = REFCHAIN(interp); - _Py_hashtable_t *main_chain = REFCHAIN(_PyInterpreterState_Main()); - if (cur_chain == main_chain) { - // It was already fixed. - return; - } - REFCHAIN(interp) = main_chain; - - (void)_Py_hashtable_foreach(cur_chain, copy_refchain_item, main_chain); - _Py_hashtable_destroy(cur_chain); -} - bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) { - maybe_fix_refchain(interp); return (_Py_hashtable_get(REFCHAIN(interp), obj) == REFCHAIN_VALUE); } @@ -272,7 +223,6 @@ _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) { - maybe_fix_refchain(interp); if (_Py_hashtable_set(REFCHAIN(interp), obj, REFCHAIN_VALUE) < 0) { // Use a fatal error because _Py_NewReference() cannot report // the error to the caller. @@ -284,7 +234,6 @@ _PyRefchain_Trace(PyInterpreterState *interp, PyObject *obj) static void _PyRefchain_Remove(PyInterpreterState *interp, PyObject *obj) { - maybe_fix_refchain(interp); void *value = _Py_hashtable_steal(REFCHAIN(interp), obj); #ifndef NDEBUG assert(value == REFCHAIN_VALUE); @@ -2630,7 +2579,6 @@ _Py_PrintReferences(PyInterpreterState *interp, FILE *fp) interp = _PyInterpreterState_Main(); } fprintf(fp, "Remaining objects:\n"); - maybe_fix_refchain(interp); _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReference, fp); } @@ -2659,7 +2607,6 @@ void _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp) { fprintf(fp, "Remaining object addresses:\n"); - maybe_fix_refchain(interp); _Py_hashtable_foreach(REFCHAIN(interp), _Py_PrintReferenceAddress, fp); } @@ -2739,7 +2686,6 @@ _Py_GetObjects(PyObject *self, PyObject *args) .limit = limit, }; PyInterpreterState *interp = _PyInterpreterState_GET(); - maybe_fix_refchain(interp); int res = _Py_hashtable_foreach(REFCHAIN(interp), _Py_GetObject, &data); if (res == _PY_GETOBJECTS_ERROR) { Py_DECREF(list); diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index b8f424854ecb86..8f38fbedae9842 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -674,6 +674,13 @@ pycore_create_interpreter(_PyRuntimeState *runtime, return status; } + // This could be done in init_interpreter() (in pystate.c) if it + // didn't depend on interp->feature_flags being set already. + status = _PyObject_InitState(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. @@ -2297,6 +2304,13 @@ new_interpreter(PyThreadState **tstate_p, goto error; } + // This could be done in init_interpreter() (in pystate.c) if it + // didn't depend on interp->feature_flags being set already. + status = _PyObject_InitState(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + // initialize the interp->obmalloc state. This must be done after // the settings are loaded (so that feature_flags are set) but before // any calls are made to obmalloc functions. diff --git a/Python/pystate.c b/Python/pystate.c index 7f734c4501ff9a..36b31f3b9e4200 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -629,13 +629,8 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; - // This relies on interp->feature_flags being set already, - // but currently we don't set that by this point. - // That's something to fix. - PyStatus status = _PyObject_InitState(interp); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + // We would call _PyObject_InitState() at this point + // if interp->feature_flags were alredy set. _PyEval_InitState(interp); _PyGC_InitState(&interp->gc); From 9d539f6715345f3821fa68ffb4ded8746aa7160f Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 22 Oct 2024 15:47:17 +0200 Subject: [PATCH 13/16] Add docs and a What's New entry --- Doc/library/sys.rst | 29 +++++++++++++++++++++++++++++ Doc/whatsnew/3.14.rst | 9 +++++++++ 2 files changed, 38 insertions(+) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 20a06a1ecd1a4c..d8df53f6c8eead 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -920,6 +920,35 @@ always available. It is not guaranteed to exist in all implementations of Python. +.. function:: getobjects(limit[, type]) + + This function only exists if CPython was built using the + specialized configure option :option:`--with-trace-refs`. + It is intended only for debugging garbage-collection issues. + + Return a list of up to *limit* dynamically allocated Python objects. + If *type* is given, only objects of that exact type (not subtypes) + are included. + + Objects from the list are not safe to use. + Specifically, the result will include objects from all interpreters that + share their object allocator state (that is, ones created with + :c:member:`PyInterpreterConfig.use_main_obmalloc` set to 0 + or using :c:func:`Py_NewInterpreter`, and the + :ref:`main interpreter `). + Mixing objects from different interpreters may lead to crashes + or other unexpected behavior. + + .. impl-detail:: + + This function should be used for specialized purposes only. + It is not guaranteed to exist in all implementations of Python. + + .. versionchanged:: next + + The result may include objects from other interpreters. + + .. function:: getprofile() .. index:: diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index feb65f244827ad..3f83e3e1e2378a 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -407,6 +407,15 @@ symtable (Contributed by Bénédikt Tran in :gh:`120029`.) + +sys +--- + +* The previously undocumented special function :func:`sys.getobjects`, + which only exists in specialized builds of Python, may now return objects + from other interpreters that the one it's called in. + + unicodedata ----------- From d80fe6ad78140fa3c58eae2994a20ec5387b8bbb Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 22 Oct 2024 14:36:17 -0600 Subject: [PATCH 14/16] Fix typos. --- Doc/library/sys.rst | 2 +- Doc/whatsnew/3.14.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index d8df53f6c8eead..37f1719db607de 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -933,7 +933,7 @@ always available. Objects from the list are not safe to use. Specifically, the result will include objects from all interpreters that share their object allocator state (that is, ones created with - :c:member:`PyInterpreterConfig.use_main_obmalloc` set to 0 + :c:member:`PyInterpreterConfig.use_main_obmalloc` set to 1 or using :c:func:`Py_NewInterpreter`, and the :ref:`main interpreter `). Mixing objects from different interpreters may lead to crashes diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index 3f83e3e1e2378a..9bba9f4298c9f6 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -413,7 +413,7 @@ sys * The previously undocumented special function :func:`sys.getobjects`, which only exists in specialized builds of Python, may now return objects - from other interpreters that the one it's called in. + from other interpreters than the one it's called in. unicodedata From 1360d86c3cb863a9c22501f31cb6e58ae6d5dbbb Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 22 Oct 2024 14:43:01 -0600 Subject: [PATCH 15/16] Do not try to destroy the refchain if it is NULL. --- Objects/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index a186f7da24e92c..7cc74a8dc0d8eb 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -207,7 +207,7 @@ refchain_init(PyInterpreterState *interp) static void refchain_fini(PyInterpreterState *interp) { - if (has_own_refchain(interp)) { + if (has_own_refchain(interp) && REFCHAIN(interp) != NULL) { _Py_hashtable_destroy(REFCHAIN(interp)); } REFCHAIN(interp) = NULL; From 090b528cf7579e1dc962ec974e6e1a05e33a8826 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 23 Oct 2024 09:52:22 +0200 Subject: [PATCH 16/16] Docs: Link to the new sys.getobjects() docs --- Doc/using/configure.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/using/configure.rst b/Doc/using/configure.rst index 10cdf2376229ff..0e7b1be5b4bc2e 100644 --- a/Doc/using/configure.rst +++ b/Doc/using/configure.rst @@ -702,7 +702,7 @@ Debug options Effects: * Define the ``Py_TRACE_REFS`` macro. - * Add :func:`!sys.getobjects` function. + * Add :func:`sys.getobjects` function. * Add :envvar:`PYTHONDUMPREFS` environment variable. The :envvar:`PYTHONDUMPREFS` environment variable can be used to dump