From 0bffa5826e459d934d6cf93f8ce4c2cf1ff64da7 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Oct 2024 02:01:57 -0700 Subject: [PATCH] gh-125286: Bug fix for trace-refs logic and shared objects. --- ...-10-11-02-26-39.gh-issue-125286.2ii5_E.rst | 3 ++ Objects/object.c | 52 ++++++++++--------- Objects/unicodeobject.c | 10 ++-- 3 files changed, 35 insertions(+), 30 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-11-02-26-39.gh-issue-125286.2ii5_E.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-11-02-26-39.gh-issue-125286.2ii5_E.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-11-02-26-39.gh-issue-125286.2ii5_E.rst new file mode 100644 index 00000000000000..5bba4483586caf --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-11-02-26-39.gh-issue-125286.2ii5_E.rst @@ -0,0 +1,3 @@ +Fix the TraceRefs build to handle objects that are shared between +interpreters (interned strings and globals of single-phase init extension +modules). diff --git a/Objects/object.c b/Objects/object.c index 4a4c5bf7d7f08a..036a80ee9cd10f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2500,38 +2500,21 @@ _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. +/* Make sure the ref is traced in the main interpreter. This is used only by + * _PyUnicode_ClearInterned() to ensure interned strings are traced. Since + * interned strings can be shared between interpreters, the interpreter that + * created the string might not be the main interpreter. We trace it here so + * that _Py_ForgetReference() will handle it without error. */ void -_Py_NormalizeImmortalReference(PyObject *op) +_Py_NormalizeReference(PyObject *op) { - assert(_Py_IsImmortal(op)); PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!_PyRefchain_IsTraced(interp, 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)); - _PyRefchain_Remove(interp, op); + if (interp == main_interp) { _PyRefchain_Trace(main_interp, op); } } @@ -2545,6 +2528,25 @@ _Py_ForgetReference(PyObject *op) PyInterpreterState *interp = _PyInterpreterState_GET(); + if (!_PyRefchain_IsTraced(interp, op)) { + if (PyUnicode_CHECK_INTERNED(op)) { + // interned strings can be shared between the main interpreter and + // between sub-interpreters due to the shared interp dict. See + // init_interned_dict(). In this case, the string was created and + // traced by a different sub-interpreter. + return; + } + PyInterpreterState *main_interp = _PyInterpreterState_Main(); + if (interp != main_interp && + interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + // objects stored in the globals of basic single-phase init + // (m_size == -1) extension modules can be shared between + // sub-interpreters. In this case, the object was created + // and traced by a different sub-interpreter. + return; + } + } + #ifdef SLOW_UNREF_CHECK if (!_PyRefchain_Get(interp, op)) { /* Not found */ diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index b94a74c2c688a9..e556e4e8ab85a8 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15445,7 +15445,7 @@ _PyUnicode_InternStatic(PyInterpreterState *interp, PyObject **p) } #ifdef Py_TRACE_REFS -extern void _Py_NormalizeImmortalReference(PyObject *); +extern void _Py_NormalizeReference(PyObject *); #endif static void @@ -15463,10 +15463,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* @@ -15678,6 +15674,10 @@ _PyUnicode_ClearInterned(PyInterpreterState *interp) while (PyDict_Next(interned, &pos, &s, &ignored_value)) { assert(PyUnicode_IS_READY(s)); int shared = 0; +#ifdef Py_TRACE_REFS + /* Make sure the ref is associated with the right interpreter. */ + _Py_NormalizeReference(s); +#endif switch (PyUnicode_CHECK_INTERNED(s)) { case SSTATE_INTERNED_IMMORTAL: /* Make immortal interned strings mortal again. */