Skip to content

Commit 6e1aa24

Browse files
committed
Fix cross-interpreter memory leak in _xidata_release via atomic claim-and-free
1 parent 07bd45d commit 6e1aa24

5 files changed

Lines changed: 105 additions & 17 deletions

File tree

Include/internal/pycore_crossinterp.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ typedef struct _xidata _PyXIData_t;
4242
typedef PyObject *(*xid_newobjfunc)(_PyXIData_t *);
4343
typedef void (*xid_freefunc)(void *);
4444

45+
typedef enum {
46+
_PyXIData_STATUS_ACTIVE = 0,
47+
_PyXIData_STATUS_CLAIMED_BY_SENDER_TEARDOWN,
48+
_PyXIData_STATUS_RELEASED,
49+
} _PyXIData_status_t;
50+
4551
// _PyXIData_t is similar to Py_buffer as an effectively
4652
// opaque struct that holds data outside the object machinery. This
4753
// is necessary to pass safely between interpreters in the same process.
@@ -82,10 +88,14 @@ struct _xidata {
8288
// to PyMem_RawFree (the default if not explicitly set to NULL).
8389
// The call will happen with the original interpreter activated.
8490
xid_freefunc free;
91+
_Py_atomic_int status; /* one of _PyXIData_status_t */
92+
struct _xidata *xid_next;
93+
struct _xidata *xid_prev;
8594
};
8695

8796
PyAPI_FUNC(_PyXIData_t *) _PyXIData_New(void);
8897
PyAPI_FUNC(void) _PyXIData_Free(_PyXIData_t *data);
98+
PyAPI_FUNC(void) _PyXIData_CleanupRegistry(PyInterpreterState *interp);
8999

90100
#define _PyXIData_DATA(DATA) ((DATA)->data)
91101
#define _PyXIData_OBJ(DATA) ((DATA)->obj)

Include/internal/pycore_interp_structs.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,9 @@ struct _is {
10691069
// guards can no longer be created.
10701070
uintptr_t finalization_guards;
10711071

1072+
PyMutex xidata_list_mutex;
1073+
struct _xidata *xidata_list_head;
1074+
10721075
/* the initial PyInterpreterState.threads.head */
10731076
_PyThreadStateImpl _initial_thread;
10741077
// _initial_thread should be the last field of PyInterpreterState.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a memory leak in :c:func:`_xidata_release` when the originating interpreter of a :c:type:`_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.

Python/crossinterp.c

Lines changed: 86 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,59 @@
1515
#include "pycore_runtime.h" // _PyRuntime
1616
#include "pycore_setobject.h" // _PySet_NextEntry()
1717
#include "pycore_typeobject.h" // _PyStaticType_InitBuiltin()
18+
static void
19+
_PyXIData_Link(PyInterpreterState *interp, _PyXIData_t *xidata)
20+
{
21+
PyMutex_Lock(&interp->xidata_list_mutex);
22+
xidata->xid_next = interp->xidata_list_head;
23+
xidata->xid_prev = NULL;
24+
if (interp->xidata_list_head != NULL) {
25+
interp->xidata_list_head->xid_prev = xidata;
26+
}
27+
interp->xidata_list_head = xidata;
28+
PyMutex_Unlock(&interp->xidata_list_mutex);
29+
}
30+
31+
static void
32+
_PyXIData_Unlink(PyInterpreterState *interp, _PyXIData_t *xidata)
33+
{
34+
PyMutex_Lock(&interp->xidata_list_mutex);
35+
if (xidata->xid_next != NULL) {
36+
xidata->xid_next->xid_prev = xidata->xid_prev;
37+
}
38+
if (xidata->xid_prev != NULL) {
39+
xidata->xid_prev->xid_next = xidata->xid_next;
40+
}
41+
else if (interp->xidata_list_head == xidata) {
42+
interp->xidata_list_head = xidata->xid_next;
43+
}
44+
xidata->xid_next = NULL;
45+
xidata->xid_prev = NULL;
46+
PyMutex_Unlock(&interp->xidata_list_mutex);
47+
}
48+
49+
void
50+
_PyXIData_CleanupRegistry(PyInterpreterState *interp)
51+
{
52+
PyMutex_Lock(&interp->xidata_list_mutex);
53+
_PyXIData_t *head = interp->xidata_list_head;
54+
interp->xidata_list_head = NULL;
55+
PyMutex_Unlock(&interp->xidata_list_mutex);
56+
57+
_PyXIData_t *curr = head;
58+
while (curr != NULL) {
59+
_PyXIData_t *next = curr->xid_next;
60+
int expected = _PyXIData_STATUS_ACTIVE;
61+
if (_Py_atomic_compare_exchange_int(&curr->status, &expected, _PyXIData_STATUS_CLAIMED_BY_SENDER_TEARDOWN)) {
62+
if (curr->free != NULL) {
63+
curr->free(curr->data);
64+
}
65+
curr->data = NULL;
66+
Py_CLEAR(curr->obj);
67+
}
68+
curr = next;
69+
}
70+
}
1871

1972

2073
static Py_ssize_t
@@ -373,6 +426,13 @@ _PyXIData_Init(_PyXIData_t *xidata,
373426
? PyInterpreterState_GetID(interp)
374427
: -1;
375428
xidata->new_object = new_object;
429+
430+
_Py_atomic_store_int(&xidata->status, _PyXIData_STATUS_ACTIVE);
431+
xidata->xid_next = NULL;
432+
xidata->xid_prev = NULL;
433+
if (interp != NULL) {
434+
_PyXIData_Link(interp, xidata);
435+
}
376436
}
377437

378438
int
@@ -1016,6 +1076,10 @@ _PyCode_GetPureScriptXIData(PyThreadState *tstate,
10161076
PyObject *
10171077
_PyXIData_NewObject(_PyXIData_t *xidata)
10181078
{
1079+
if (_Py_atomic_load_int(&xidata->status) != _PyXIData_STATUS_ACTIVE) {
1080+
PyErr_SetString(PyExc_RuntimeError, "the originating interpreter has been destroyed");
1081+
return NULL;
1082+
}
10191083
return xidata->new_object(xidata);
10201084
}
10211085

@@ -1040,26 +1104,31 @@ _xidata_release(_PyXIData_t *xidata, int rawfree)
10401104
return 0;
10411105
}
10421106

1043-
// Switch to the original interpreter.
1044-
PyInterpreterState *interp = _PyInterpreterState_LookUpID(
1045-
_PyXIData_INTERPID(xidata));
1046-
if (interp == NULL) {
1047-
// The interpreter was already destroyed.
1048-
// This function shouldn't have been called.
1049-
// XXX Someone leaked some memory...
1050-
assert(PyErr_Occurred());
1051-
if (rawfree) {
1052-
PyMem_RawFree(xidata);
1107+
int expected = _PyXIData_STATUS_ACTIVE;
1108+
if (_Py_atomic_compare_exchange_int(&xidata->status, &expected, _PyXIData_STATUS_RELEASED)) {
1109+
PyInterpreterState *interp = _PyInterpreterState_LookUpID(_PyXIData_INTERPID(xidata));
1110+
if (interp != NULL) {
1111+
_PyXIData_Unlink(interp, xidata);
1112+
if (rawfree) {
1113+
return _Py_CallInInterpreterAndRawFree(interp, _call_clear_xidata, xidata);
1114+
}
1115+
else {
1116+
return _Py_CallInInterpreter(interp, _call_clear_xidata, xidata);
1117+
}
1118+
}
1119+
else {
1120+
if (rawfree) {
1121+
PyMem_RawFree(xidata);
1122+
}
1123+
return -1;
10531124
}
1054-
return -1;
1055-
}
1056-
1057-
// "Release" the data and/or the object.
1058-
if (rawfree) {
1059-
return _Py_CallInInterpreterAndRawFree(interp, _call_clear_xidata, xidata);
10601125
}
10611126
else {
1062-
return _Py_CallInInterpreter(interp, _call_clear_xidata, xidata);
1127+
// Already claimed by sender teardown.
1128+
if (rawfree) {
1129+
PyMem_RawFree(xidata);
1130+
}
1131+
return 0;
10631132
}
10641133
}
10651134

Python/pystate.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "pycore_ceval.h" // _PyEval_AcquireLock()
99
#include "pycore_codecs.h" // _PyCodec_Fini()
1010
#include "pycore_critical_section.h" // _PyCriticalSection_Resume()
11+
#include "pycore_crossinterp.h" // _PyXIData_CleanupRegistry()
1112
#include "pycore_dtoa.h" // _dtoa_state_INIT()
1213
#include "pycore_freelist.h" // _PyObject_ClearFreeLists()
1314
#include "pycore_initconfig.h" // _PyStatus_OK()
@@ -598,6 +599,8 @@ init_interpreter(PyInterpreterState *interp,
598599
interp->executor_ptrs = NULL;
599600
interp->executor_count = 0;
600601
interp->executor_capacity = 0;
602+
interp->xidata_list_mutex = (PyMutex){0};
603+
interp->xidata_list_head = NULL;
601604
interp->executor_deletion_list_head = NULL;
602605
interp->executor_creation_counter = JIT_CLEANUP_THRESHOLD;
603606

@@ -1066,6 +1069,8 @@ PyInterpreterState_Delete(PyInterpreterState *interp)
10661069

10671070
PyConfig_Clear(&interp->config);
10681071

1072+
_PyXIData_CleanupRegistry(interp);
1073+
10691074
free_interpreter(interp);
10701075
}
10711076

0 commit comments

Comments
 (0)