gh-107080: Fix Py_TRACE_REFS Crashes Under Isolated Subinterpreters (gh-107567)

The linked list of objects was a global variable, which broke isolation between interpreters, causing crashes. To solve this, we've moved the linked list to each interpreter.
This commit is contained in:
Eric Snow 2023-08-03 13:51:08 -06:00 committed by GitHub
parent 14fbd4e6b1
commit 58ef741867
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 62 additions and 29 deletions

View file

@ -292,8 +292,8 @@ extern void _PyDebug_PrintTotalRefs(void);
#ifdef Py_TRACE_REFS #ifdef Py_TRACE_REFS
extern void _Py_AddToAllObjects(PyObject *op, int force); extern void _Py_AddToAllObjects(PyObject *op, int force);
extern void _Py_PrintReferences(FILE *); extern void _Py_PrintReferences(PyInterpreterState *, FILE *);
extern void _Py_PrintReferenceAddresses(FILE *); extern void _Py_PrintReferenceAddresses(PyInterpreterState *, FILE *);
#endif #endif

View file

@ -11,17 +11,22 @@ extern "C" {
struct _py_object_runtime_state { struct _py_object_runtime_state {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
Py_ssize_t interpreter_leaks; Py_ssize_t interpreter_leaks;
#else
int _not_used;
#endif #endif
int _not_used;
}; };
struct _py_object_state { struct _py_object_state {
#ifdef Py_REF_DEBUG #ifdef Py_REF_DEBUG
Py_ssize_t reftotal; Py_ssize_t reftotal;
#else
int _not_used;
#endif #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;
}; };

View file

@ -157,6 +157,7 @@ extern PyTypeObject _PyExc_MemoryError;
{ .threshold = 10, }, \ { .threshold = 10, }, \
}, \ }, \
}, \ }, \
.object_state = _py_object_state_INIT(INTERP), \
.dtoa = _dtoa_state_INIT(&(INTERP)), \ .dtoa = _dtoa_state_INIT(&(INTERP)), \
.dict_state = _dict_state_INIT, \ .dict_state = _dict_state_INIT, \
.func_state = { \ .func_state = { \
@ -186,6 +187,16 @@ extern PyTypeObject _PyExc_MemoryError;
.context_ver = 1, \ .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 // global objects

View file

@ -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.

View file

@ -159,11 +159,8 @@ _PyDebug_PrintTotalRefs(void) {
Do not call them otherwise, they do not initialize the object! */ Do not call them otherwise, they do not initialize the object! */
#ifdef Py_TRACE_REFS #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 #define REFCHAIN(interp) &interp->object_state.refchain
* exist only in a Py_TRACE_REFS build.
*/
static PyObject refchain = {&refchain, &refchain};
/* Insert op at the front of the list of all objects. If force is true, /* 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 * op is added even if _ob_prev and _ob_next are non-NULL already. If
@ -188,10 +185,11 @@ _Py_AddToAllObjects(PyObject *op, int force)
} }
#endif #endif
if (force || op->_ob_prev == NULL) { if (force || op->_ob_prev == NULL) {
op->_ob_next = refchain._ob_next; PyObject *refchain = REFCHAIN(_PyInterpreterState_GET());
op->_ob_prev = &refchain; op->_ob_next = refchain->_ob_next;
refchain._ob_next->_ob_prev = op; op->_ob_prev = refchain;
refchain._ob_next = op; refchain->_ob_next->_ob_prev = op;
refchain->_ob_next = op;
} }
} }
#endif /* Py_TRACE_REFS */ #endif /* Py_TRACE_REFS */
@ -2229,7 +2227,8 @@ _Py_ForgetReference(PyObject *op)
_PyObject_ASSERT_FAILED_MSG(op, "negative refcnt"); _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) op->_ob_prev->_ob_next != op || op->_ob_next->_ob_prev != op)
{ {
_PyObject_ASSERT_FAILED_MSG(op, "invalid object chain"); _PyObject_ASSERT_FAILED_MSG(op, "invalid object chain");
@ -2237,12 +2236,12 @@ _Py_ForgetReference(PyObject *op)
#ifdef SLOW_UNREF_CHECK #ifdef SLOW_UNREF_CHECK
PyObject *p; 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) { if (p == op) {
break; break;
} }
} }
if (p == &refchain) { if (p == refchain) {
/* Not found */ /* Not found */
_PyObject_ASSERT_FAILED_MSG(op, _PyObject_ASSERT_FAILED_MSG(op,
"object not found in the objects list"); "object not found in the objects list");
@ -2258,11 +2257,15 @@ _Py_ForgetReference(PyObject *op)
* interpreter must be in a healthy state. * interpreter must be in a healthy state.
*/ */
void void
_Py_PrintReferences(FILE *fp) _Py_PrintReferences(PyInterpreterState *interp, FILE *fp)
{ {
PyObject *op; PyObject *op;
if (interp == NULL) {
interp = _PyInterpreterState_Main();
}
fprintf(fp, "Remaining objects:\n"); 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)); fprintf(fp, "%p [%zd] ", (void *)op, Py_REFCNT(op));
if (PyObject_Print(op, fp, 0) != 0) { if (PyObject_Print(op, fp, 0) != 0) {
PyErr_Clear(); PyErr_Clear();
@ -2274,34 +2277,42 @@ _Py_PrintReferences(FILE *fp)
/* Print the addresses of all live objects. Unlike _Py_PrintReferences, this /* 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. * 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 void
_Py_PrintReferenceAddresses(FILE *fp) _Py_PrintReferenceAddresses(PyInterpreterState *interp, FILE *fp)
{ {
PyObject *op; PyObject *op;
PyObject *refchain = REFCHAIN(interp);
fprintf(fp, "Remaining object addresses:\n"); 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, fprintf(fp, "%p [%zd] %s\n", (void *)op,
Py_REFCNT(op), Py_TYPE(op)->tp_name); Py_REFCNT(op), Py_TYPE(op)->tp_name);
} }
/* The implementation of sys.getobjects(). */
PyObject * PyObject *
_Py_GetObjects(PyObject *self, PyObject *args) _Py_GetObjects(PyObject *self, PyObject *args)
{ {
int i, n; int i, n;
PyObject *t = NULL; PyObject *t = NULL;
PyObject *res, *op; PyObject *res, *op;
PyInterpreterState *interp = _PyInterpreterState_GET();
if (!PyArg_ParseTuple(args, "i|O", &n, &t)) if (!PyArg_ParseTuple(args, "i|O", &n, &t))
return NULL; return NULL;
op = refchain._ob_next; PyObject *refchain = REFCHAIN(interp);
op = refchain->_ob_next;
res = PyList_New(0); res = PyList_New(0);
if (res == NULL) if (res == NULL)
return 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 || while (op == self || op == args || op == res || op == t ||
(t != NULL && !Py_IS_TYPE(op, (PyTypeObject *) t))) { (t != NULL && !Py_IS_TYPE(op, (PyTypeObject *) t))) {
op = op->_ob_next; op = op->_ob_next;
if (op == &refchain) if (op == refchain)
return res; return res;
} }
if (PyList_Append(res, op) < 0) { if (PyList_Append(res, op) < 0) {
@ -2313,6 +2324,8 @@ _Py_GetObjects(PyObject *self, PyObject *args)
return res; return res;
} }
#undef REFCHAIN
#endif #endif

View file

@ -1921,11 +1921,11 @@ Py_FinalizeEx(void)
} }
if (dump_refs) { if (dump_refs) {
_Py_PrintReferences(stderr); _Py_PrintReferences(tstate->interp, stderr);
} }
if (dump_refs_fp != NULL) { if (dump_refs_fp != NULL) {
_Py_PrintReferences(dump_refs_fp); _Py_PrintReferences(tstate->interp, dump_refs_fp);
} }
#endif /* Py_TRACE_REFS */ #endif /* Py_TRACE_REFS */
@ -1961,11 +1961,11 @@ Py_FinalizeEx(void)
*/ */
if (dump_refs) { if (dump_refs) {
_Py_PrintReferenceAddresses(stderr); _Py_PrintReferenceAddresses(tstate->interp, stderr);
} }
if (dump_refs_fp != NULL) { if (dump_refs_fp != NULL) {
_Py_PrintReferenceAddresses(dump_refs_fp); _Py_PrintReferenceAddresses(tstate->interp, dump_refs_fp);
fclose(dump_refs_fp); fclose(dump_refs_fp);
} }
#endif /* Py_TRACE_REFS */ #endif /* Py_TRACE_REFS */