[Python-checkins] [3.12] gh-107080: Fix Py_TRACE_REFS Crashes Under Isolated Subinterpreters (#107751)
Yhg1s
webhook-mailer at python.org
Wed Aug 16 06:03:09 EDT 2023
https://github.com/python/cpython/commit/aa9707dda9f8dcbe2ada8d8a7280f0f6a8229c59
commit: aa9707dda9f8dcbe2ada8d8a7280f0f6a8229c59
branch: 3.12
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: Yhg1s <thomas at python.org>
date: 2023-08-16T12:03:05+02:00
summary:
[3.12] gh-107080: Fix Py_TRACE_REFS Crashes Under Isolated Subinterpreters (#107751)
* Unrevert "[3.12] gh-107080: Fix Py_TRACE_REFS Crashes Under Isolated Subinterpreters (gh-107567) (#107599)".
This reverts commit 6e4eec760648a71e1cd8f8f551997b1823b4bb9f (gh-107648).
* Initialize each interpreter's refchain properly.
* Skip test_basic_multiple_interpreters_deleted_no_reset on tracerefs builds.
files:
A Misc/NEWS.d/next/Core and Builtins/2023-08-02-12-24-51.gh-issue-107080.PNolFU.rst
M Include/internal/pycore_object.h
M Include/internal/pycore_object_state.h
M Include/internal/pycore_runtime_init.h
M Lib/test/test_import/__init__.py
M Objects/object.c
M Python/pylifecycle.c
M Python/pystate.c
diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h
index 0981d1122fec5..7a2f13a21bda7 100644
--- a/Include/internal/pycore_object.h
+++ b/Include/internal/pycore_object.h
@@ -152,6 +152,7 @@ _PyType_HasFeature(PyTypeObject *type, unsigned long feature) {
extern void _PyType_InitCache(PyInterpreterState *interp);
+extern void _PyObject_InitState(PyInterpreterState *interp);
/* Inline functions trading binary compatibility for speed:
_PyObject_Init() is the fast version of PyObject_Init(), and
@@ -271,8 +272,8 @@ 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(PyInterpreterState *, FILE *);
#endif
diff --git a/Include/internal/pycore_object_state.h b/Include/internal/pycore_object_state.h
index 94005d7788143..65feb5af969f8 100644
--- a/Include/internal/pycore_object_state.h
+++ b/Include/internal/pycore_object_state.h
@@ -11,17 +11,22 @@ extern "C" {
struct _py_object_runtime_state {
#ifdef Py_REF_DEBUG
Py_ssize_t interpreter_leaks;
-#else
- int _not_used;
#endif
+ int _not_used;
};
struct _py_object_state {
#ifdef Py_REF_DEBUG
Py_ssize_t reftotal;
-#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;
};
diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h
index 4130188079cff..7aace9f86119c 100644
--- a/Include/internal/pycore_runtime_init.h
+++ b/Include/internal/pycore_runtime_init.h
@@ -101,6 +101,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 = { \
@@ -130,6 +131,16 @@ extern PyTypeObject _PyExc_MemoryError;
.context_ver = 1, \
}
+#ifdef Py_TRACE_REFS
+# define _py_object_state_INIT(INTERP) \
+ { \
+ .refchain = {&INTERP.object_state.refchain, &INTERP.object_state.refchain}, \
+ }
+#else
+# define _py_object_state_INIT(INTERP) \
+ { 0 }
+#endif
+
// global objects
diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index 06adf01f18c0a..f7df6d7e42319 100644
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -2539,6 +2539,12 @@ def test_basic_multiple_interpreters_main_no_reset(self):
def test_basic_multiple_interpreters_deleted_no_reset(self):
# without resetting; already loaded in a deleted interpreter
+ if hasattr(sys, 'getobjects'):
+ # It's a Py_TRACE_REFS build.
+ # This test breaks interpreter isolation a little,
+ # which causes problems on Py_TRACE_REF builds.
+ raise unittest.SkipTest('crashes on Py_TRACE_REFS builds')
+
# At this point:
# * alive in 0 interpreters
# * module def may or may not be loaded already
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 0000000000000..5084c854360e3
--- /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.
diff --git a/Objects/object.c b/Objects/object.c
index bd0fa40bd2a85..8a4010fb13669 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -158,11 +158,16 @@ _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(interp) &interp->object_state.refchain
+
+static inline void
+init_refchain(PyInterpreterState *interp)
+{
+ PyObject *refchain = REFCHAIN(interp);
+ refchain->_ob_prev = refchain;
+ refchain->_ob_next = 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
@@ -187,10 +192,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 */
@@ -1998,6 +2004,18 @@ PyObject _Py_NotImplementedStruct = {
&_PyNotImplemented_Type
};
+
+void
+_PyObject_InitState(PyInterpreterState *interp)
+{
+#ifdef Py_TRACE_REFS
+ if (!_Py_IsMainInterpreter(interp)) {
+ init_refchain(interp);
+ }
+#endif
+}
+
+
extern PyTypeObject _Py_GenericAliasIterType;
extern PyTypeObject _PyMemoryIter_Type;
extern PyTypeObject _PyLineIterator;
@@ -2206,7 +2224,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");
@@ -2214,12 +2233,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");
@@ -2235,11 +2254,15 @@ _Py_ForgetReference(PyObject *op)
* 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();
@@ -2251,34 +2274,42 @@ _Py_PrintReferences(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(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)
+ 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)
{
int i, n;
PyObject *t = NULL;
PyObject *res, *op;
+ PyInterpreterState *interp = _PyInterpreterState_GET();
if (!PyArg_ParseTuple(args, "i|O", &n, &t))
return NULL;
- op = refchain._ob_next;
+ PyObject *refchain = REFCHAIN(interp);
+ 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) {
@@ -2290,7 +2321,9 @@ _Py_GetObjects(PyObject *self, PyObject *args)
return res;
}
-#endif
+#undef REFCHAIN
+
+#endif /* Py_TRACE_REFS */
/* Hack to force loading of abstract.o */
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index a67fa26b37227..29771e07ae6a2 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1920,11 +1920,11 @@ 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);
}
#endif /* Py_TRACE_REFS */
@@ -1960,11 +1960,11 @@ Py_FinalizeEx(void)
*/
if (dump_refs) {
- _Py_PrintReferenceAddresses(stderr);
+ _Py_PrintReferenceAddresses(tstate->interp, stderr);
}
if (dump_refs_fp != NULL) {
- _Py_PrintReferenceAddresses(dump_refs_fp);
+ _Py_PrintReferenceAddresses(tstate->interp, dump_refs_fp);
fclose(dump_refs_fp);
}
#endif /* Py_TRACE_REFS */
@@ -2074,6 +2074,8 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
}
has_gil = 1;
+ /* No objects have been created yet. */
+
status = pycore_interp_init(tstate);
if (_PyStatus_EXCEPTION(status)) {
goto error;
diff --git a/Python/pystate.c b/Python/pystate.c
index 8097124965cb7..2ee16e3de25da 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -673,6 +673,7 @@ init_interpreter(PyInterpreterState *interp,
_obmalloc_pools_INIT(interp->obmalloc.pools);
memcpy(&interp->obmalloc.pools.used, temp, sizeof(temp));
}
+ _PyObject_InitState(interp);
_PyEval_InitState(interp, pending_lock);
_PyGC_InitState(&interp->gc);
More information about the Python-checkins
mailing list