Skip to content

[3.12] gh-107080: Fix Py_TRACE_REFS Crashes Under Isolated Subinterpreters #107751

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 3 commits into from
Aug 16, 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
5 changes: 3 additions & 2 deletions Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


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 @@ -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 = { \
Expand Down Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
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.
73 changes: 53 additions & 20 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2206,20 +2224,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 @@ -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();
Expand All @@ -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) {
Expand All @@ -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 */
Expand Down
10 changes: 6 additions & 4 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down