gh-76785: Use Pending Calls When Releasing Cross-Interpreter Data (gh… · python/cpython@fd7e08a · GitHub
Skip to content

Commit fd7e08a

Browse files
ericsnowcurrentlyericsnowcurrently
authored
gh-76785: Use Pending Calls When Releasing Cross-Interpreter Data (gh-109556)
This fixes some crashes in the _xxinterpchannels module, due to a race between interpreters.
1 parent 754519a commit fd7e08a

7 files changed

Lines changed: 98 additions & 67 deletions

File tree

‎Include/cpython/pystate.h‎

Lines changed: 1 addition & 0 deletions

‎Include/internal/pycore_ceval.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ extern void _PyEval_SignalReceived(PyInterpreterState *interp);
4444
// Export for '_testinternalcapi' shared extension
4545
PyAPI_FUNC(int) _PyEval_AddPendingCall(
4646
PyInterpreterState *interp,
47-
int (*func)(void *),
47+
_Py_pending_call_func func,
4848
void *arg,
4949
int mainthreadonly);
5050

‎Include/internal/pycore_ceval_state.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ extern "C" {
1111
#include "pycore_gil.h" // struct _gil_runtime_state
1212

1313

14+
typedef int (*_Py_pending_call_func)(void *);
15+
1416
struct _pending_calls {
1517
int busy;
1618
PyThread_type_lock lock;
@@ -22,7 +24,7 @@ struct _pending_calls {
2224
int async_exc;
2325
#define NPENDINGCALLS 32
2426
struct _pending_call {
25-
int (*func)(void *);
27+
_Py_pending_call_func func;
2628
void *arg;
2729
} calls[NPENDINGCALLS];
2830
int first;

‎Modules/_xxinterpchannelsmodule.c‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,21 +160,34 @@ add_new_type(PyObject *mod, PyType_Spec *spec, crossinterpdatafunc shared)
160160
return cls;
161161
}
162162

163+
#define XID_IGNORE_EXC 1
164+
#define XID_FREE 2
165+
163166
static int
164-
_release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
167+
_release_xid_data(_PyCrossInterpreterData *data, int flags)
165168
{
169+
int ignoreexc = flags & XID_IGNORE_EXC;
166170
PyObject *exc;
167171
if (ignoreexc) {
168172
exc = PyErr_GetRaisedException();
169173
}
170-
int res = _PyCrossInterpreterData_Release(data);
174+
int res;
175+
if (flags & XID_FREE) {
176+
res = _PyCrossInterpreterData_ReleaseAndRawFree(data);
177+
}
178+
else {
179+
res = _PyCrossInterpreterData_Release(data);
180+
}
171181
if (res < 0) {
172182
/* The owning interpreter is already destroyed. */
173183
if (ignoreexc) {
174184
// XXX Emit a warning?
175185
PyErr_Clear();
176186
}
177187
}
188+
if (flags & XID_FREE) {
189+
/* Either way, we free the data. */
190+
}
178191
if (ignoreexc) {
179192
PyErr_SetRaisedException(exc);
180193
}
@@ -366,9 +379,8 @@ static void
366379
_channelitem_clear(_channelitem *item)
367380
{
368381
if (item->data != NULL) {
369-
(void)_release_xid_data(item->data, 1);
370382
// It was allocated in _channel_send().
371-
GLOBAL_FREE(item->data);
383+
(void)_release_xid_data(item->data, XID_IGNORE_EXC & XID_FREE);
372384
item->data = NULL;
373385
}
374386
item->next = NULL;
@@ -1439,14 +1451,12 @@ _channel_recv(_channels *channels, int64_t id, PyObject **res)
14391451
PyObject *obj = _PyCrossInterpreterData_NewObject(data);
14401452
if (obj == NULL) {
14411453
assert(PyErr_Occurred());
1442-
(void)_release_xid_data(data, 1);
1443-
// It was allocated in _channel_send().
1444-
GLOBAL_FREE(data);
1454+
// It was allocated in _channel_send(), so we free it.
1455+
(void)_release_xid_data(data, XID_IGNORE_EXC | XID_FREE);
14451456
return -1;
14461457
}
1447-
int release_res = _release_xid_data(data, 0);
1448-
// It was allocated in _channel_send().
1449-
GLOBAL_FREE(data);
1458+
// It was allocated in _channel_send(), so we free it.
1459+
int release_res = _release_xid_data(data, XID_FREE);
14501460
if (release_res < 0) {
14511461
// The source interpreter has been destroyed already.
14521462
assert(PyErr_Occurred());

‎Modules/_xxsubinterpretersmodule.c‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -58,24 +58,17 @@ add_new_exception(PyObject *mod, const char *name, PyObject *base)
5858
add_new_exception(MOD, MODULE_NAME "." Py_STRINGIFY(NAME), BASE)
5959

6060
static int
61-
_release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
61+
_release_xid_data(_PyCrossInterpreterData *data)
6262
{
63-
PyObject *exc;
64-
if (ignoreexc) {
65-
exc = PyErr_GetRaisedException();
66-
}
63+
PyObject *exc = PyErr_GetRaisedException();
6764
int res = _PyCrossInterpreterData_Release(data);
6865
if (res < 0) {
6966
/* The owning interpreter is already destroyed. */
7067
_PyCrossInterpreterData_Clear(NULL, data);
71-
if (ignoreexc) {
72-
// XXX Emit a warning?
73-
PyErr_Clear();
74-
}
75-
}
76-
if (ignoreexc) {
77-
PyErr_SetRaisedException(exc);
68+
// XXX Emit a warning?
69+
PyErr_Clear();
7870
}
71+
PyErr_SetRaisedException(exc);
7972
return res;
8073
}
8174

@@ -145,7 +138,7 @@ _sharednsitem_clear(struct _sharednsitem *item)
145138
PyMem_RawFree((void *)item->name);
146139
item->name = NULL;
147140
}
148-
(void)_release_xid_data(&item->data, 1);
141+
(void)_release_xid_data(&item->data);
149142
}
150143

151144
static int
@@ -174,16 +167,16 @@ typedef struct _sharedns {
174167
static _sharedns *
175168
_sharedns_new(Py_ssize_t len)
176169
{
177-
_sharedns *shared = PyMem_NEW(_sharedns, 1);
170+
_sharedns *shared = PyMem_RawCalloc(sizeof(_sharedns), 1);
178171
if (shared == NULL) {
179172
PyErr_NoMemory();
180173
return NULL;
181174
}
182175
shared->len = len;
183-
shared->items = PyMem_NEW(struct _sharednsitem, len);
176+
shared->items = PyMem_RawCalloc(sizeof(struct _sharednsitem), len);
184177
if (shared->items == NULL) {
185178
PyErr_NoMemory();
186-
PyMem_Free(shared);
179+
PyMem_RawFree(shared);
187180
return NULL;
188181
}
189182
return shared;
@@ -195,8 +188,8 @@ _sharedns_free(_sharedns *shared)
195188
for (Py_ssize_t i=0; i < shared->len; i++) {
196189
_sharednsitem_clear(&shared->items[i]);
197190
}
198-
PyMem_Free(shared->items);
199-
PyMem_Free(shared);
191+
PyMem_RawFree(shared->items);
192+
PyMem_RawFree(shared);
200193
}
201194

202195
static _sharedns *

‎Python/ceval_gil.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ _PyEval_SignalReceived(PyInterpreterState *interp)
763763
/* Push one item onto the queue while holding the lock. */
764764
static int
765765
_push_pending_call(struct _pending_calls *pending,
766-
int (*func)(void *), void *arg)
766+
_Py_pending_call_func func, void *arg)
767767
{
768768
int i = pending->last;
769769
int j = (i + 1) % NPENDINGCALLS;
@@ -810,7 +810,7 @@ _pop_pending_call(struct _pending_calls *pending,
810810

811811
int
812812
_PyEval_AddPendingCall(PyInterpreterState *interp,
813-
int (*func)(void *), void *arg,
813+
_Py_pending_call_func func, void *arg,
814814
int mainthreadonly)
815815
{
816816
assert(!mainthreadonly || _Py_IsMainInterpreter(interp));
@@ -834,7 +834,7 @@ _PyEval_AddPendingCall(PyInterpreterState *interp,
834834
}
835835

836836
int
837-
Py_AddPendingCall(int (*func)(void *), void *arg)
837+
Py_AddPendingCall(_Py_pending_call_func func, void *arg)
838838
{
839839
/* Legacy users of this API will continue to target the main thread
840840
(of the main interpreter). */
@@ -878,7 +878,7 @@ _make_pending_calls(struct _pending_calls *pending)
878878
{
879879
/* perform a bounded number of calls, in case of recursion */
880880
for (int i=0; i<NPENDINGCALLS; i++) {
881-
int (*func)(void *) = NULL;
881+
_Py_pending_call_func func = NULL;
882882
void *arg = NULL;
883883

884884
/* pop one item off the queue while holding the lock */

‎Python/pystate.c‎

Lines changed: 58 additions & 33 deletions

0 commit comments

Comments
 (0)