Skip to content
Closed
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
10 changes: 10 additions & 0 deletions Include/internal/pycore_crossinterp.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ typedef struct _xidata _PyXIData_t;
typedef PyObject *(*xid_newobjfunc)(_PyXIData_t *);
typedef void (*xid_freefunc)(void *);

typedef enum {
_PyXIData_STATUS_ACTIVE = 0,
_PyXIData_STATUS_CLAIMED_BY_SENDER_TEARDOWN,
_PyXIData_STATUS_RELEASED,
} _PyXIData_status_t;

// _PyXIData_t is similar to Py_buffer as an effectively
// opaque struct that holds data outside the object machinery. This
// is necessary to pass safely between interpreters in the same process.
Expand Down Expand Up @@ -82,10 +88,14 @@ struct _xidata {
// to PyMem_RawFree (the default if not explicitly set to NULL).
// The call will happen with the original interpreter activated.
xid_freefunc free;
int status; /* one of _PyXIData_status_t */
struct _xidata *xid_next;
struct _xidata *xid_prev;
};

PyAPI_FUNC(_PyXIData_t *) _PyXIData_New(void);
PyAPI_FUNC(void) _PyXIData_Free(_PyXIData_t *data);
PyAPI_FUNC(void) _PyXIData_CleanupRegistry(PyInterpreterState *interp);

#define _PyXIData_DATA(DATA) ((DATA)->data)
#define _PyXIData_OBJ(DATA) ((DATA)->obj)
Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_interp_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,9 @@ struct _is {
// guards can no longer be created.
uintptr_t finalization_guards;

PyMutex xidata_list_mutex;
struct _xidata *xidata_list_head;

/* the initial PyInterpreterState.threads.head */
_PyThreadStateImpl _initial_thread;
// _initial_thread should be the last field of PyInterpreterState.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a memory leak in ``_xidata_release`` when the originating interpreter of a ``_PyXIData_t`` object is destroyed before the receiving interpreter releases it. The fix implements a lock-free compare-and-swap mechanism to safely coordinate teardown and release across interpreters.
106 changes: 89 additions & 17 deletions Python/crossinterp.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,62 @@
#include "pycore_runtime.h" // _PyRuntime
#include "pycore_setobject.h" // _PySet_NextEntry()
#include "pycore_typeobject.h" // _PyStaticType_InitBuiltin()
static void
_PyXIData_Link(PyInterpreterState *interp, _PyXIData_t *xidata)
{
PyMutex_Lock(&interp->xidata_list_mutex);
xidata->xid_next = interp->xidata_list_head;
xidata->xid_prev = NULL;
if (interp->xidata_list_head != NULL) {
interp->xidata_list_head->xid_prev = xidata;
}
interp->xidata_list_head = xidata;
PyMutex_Unlock(&interp->xidata_list_mutex);
}

static void
_PyXIData_Unlink(PyInterpreterState *interp, _PyXIData_t *xidata)
{
PyMutex_Lock(&interp->xidata_list_mutex);
if (xidata->xid_next != NULL) {
xidata->xid_next->xid_prev = xidata->xid_prev;
}
if (xidata->xid_prev != NULL) {
xidata->xid_prev->xid_next = xidata->xid_next;
}
else if (interp->xidata_list_head == xidata) {
interp->xidata_list_head = xidata->xid_next;
}
xidata->xid_next = NULL;
xidata->xid_prev = NULL;
PyMutex_Unlock(&interp->xidata_list_mutex);
}

void
_PyXIData_CleanupRegistry(PyInterpreterState *interp)
{
PyMutex_Lock(&interp->xidata_list_mutex);
_PyXIData_t *head = interp->xidata_list_head;
interp->xidata_list_head = NULL;
PyMutex_Unlock(&interp->xidata_list_mutex);

_PyXIData_t *curr = head;
while (curr != NULL) {
_PyXIData_t *next = curr->xid_next;
void *data = curr->data;
xid_freefunc free_func = curr->free;
PyObject *obj = curr->obj;

int expected = _PyXIData_STATUS_ACTIVE;
if (_Py_atomic_compare_exchange_int(&curr->status, &expected, _PyXIData_STATUS_CLAIMED_BY_SENDER_TEARDOWN)) {
if (free_func != NULL) {
free_func(data);
}
Py_XDECREF(obj);
}
curr = next;
}
}


static Py_ssize_t
Expand Down Expand Up @@ -373,6 +429,13 @@ _PyXIData_Init(_PyXIData_t *xidata,
? PyInterpreterState_GetID(interp)
: -1;
xidata->new_object = new_object;

_Py_atomic_store_int(&xidata->status, _PyXIData_STATUS_ACTIVE);
xidata->xid_next = NULL;
xidata->xid_prev = NULL;
if (interp != NULL) {
_PyXIData_Link(interp, xidata);
}
}

int
Expand Down Expand Up @@ -1016,6 +1079,10 @@ _PyCode_GetPureScriptXIData(PyThreadState *tstate,
PyObject *
_PyXIData_NewObject(_PyXIData_t *xidata)
{
if (_Py_atomic_load_int(&xidata->status) != _PyXIData_STATUS_ACTIVE) {
PyErr_SetString(PyExc_RuntimeError, "the originating interpreter has been destroyed");
return NULL;
}
return xidata->new_object(xidata);
}

Expand All @@ -1040,26 +1107,31 @@ _xidata_release(_PyXIData_t *xidata, int rawfree)
return 0;
}

// Switch to the original interpreter.
PyInterpreterState *interp = _PyInterpreterState_LookUpID(
_PyXIData_INTERPID(xidata));
if (interp == NULL) {
// The interpreter was already destroyed.
// This function shouldn't have been called.
// XXX Someone leaked some memory...
assert(PyErr_Occurred());
if (rawfree) {
PyMem_RawFree(xidata);
int expected = _PyXIData_STATUS_ACTIVE;
if (_Py_atomic_compare_exchange_int(&xidata->status, &expected, _PyXIData_STATUS_RELEASED)) {
PyInterpreterState *interp = _PyInterpreterState_LookUpID(_PyXIData_INTERPID(xidata));
if (interp != NULL) {
_PyXIData_Unlink(interp, xidata);
if (rawfree) {
return _Py_CallInInterpreterAndRawFree(interp, _call_clear_xidata, xidata);
}
else {
return _Py_CallInInterpreter(interp, _call_clear_xidata, xidata);
}
}
else {
if (rawfree) {
PyMem_RawFree(xidata);
}
return -1;
}
return -1;
}

// "Release" the data and/or the object.
if (rawfree) {
return _Py_CallInInterpreterAndRawFree(interp, _call_clear_xidata, xidata);
}
else {
return _Py_CallInInterpreter(interp, _call_clear_xidata, xidata);
// Already claimed by sender teardown.
if (rawfree) {
PyMem_RawFree(xidata);
}
return 0;
}
}

Expand Down
5 changes: 5 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "pycore_ceval.h" // _PyEval_AcquireLock()
#include "pycore_codecs.h" // _PyCodec_Fini()
#include "pycore_critical_section.h" // _PyCriticalSection_Resume()
#include "pycore_crossinterp.h" // _PyXIData_CleanupRegistry()
#include "pycore_dtoa.h" // _dtoa_state_INIT()
#include "pycore_freelist.h" // _PyObject_ClearFreeLists()
#include "pycore_initconfig.h" // _PyStatus_OK()
Expand Down Expand Up @@ -598,6 +599,8 @@ init_interpreter(PyInterpreterState *interp,
interp->executor_ptrs = NULL;
interp->executor_count = 0;
interp->executor_capacity = 0;
interp->xidata_list_mutex = (PyMutex){0};
interp->xidata_list_head = NULL;
interp->executor_deletion_list_head = NULL;
interp->executor_creation_counter = JIT_CLEANUP_THRESHOLD;

Expand Down Expand Up @@ -1030,6 +1033,8 @@ PyInterpreterState_Delete(PyInterpreterState *interp)

zapthreads(interp);

_PyXIData_CleanupRegistry(interp);

// XXX These two calls should be done at the end of clear_interpreter(),
// but currently some objects get decref'ed after that.
#ifdef Py_REF_DEBUG
Expand Down
Loading