Skip to content

gh-107080: Fix Py_TRACE_REFS Crashes Under Isolated Subinterpreters #107567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,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


Expand Down
13 changes: 9 additions & 4 deletions Include/internal/pycore_object_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};


Expand Down
11 changes: 11 additions & 0 deletions Include/internal/pycore_runtime_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,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 = { \
Expand Down Expand Up @@ -186,6 +187,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

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
51 changes: 32 additions & 19 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(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
Expand All @@ -188,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 */
Expand Down Expand Up @@ -2229,20 +2227,21 @@ _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");
}

#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");
Expand All @@ -2258,11 +2257,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();
Expand All @@ -2274,34 +2277,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) {
Expand All @@ -2313,6 +2324,8 @@ _Py_GetObjects(PyObject *self, PyObject *args)
return res;
}

#undef REFCHAIN

#endif


Expand Down
8 changes: 4 additions & 4 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1921,11 +1921,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 */

Expand Down Expand Up @@ -1961,11 +1961,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 */
Expand Down