Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

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.
This commit is contained in:
Eric Snow 2023-09-19 15:01:34 -06:00 committed by GitHub
parent 754519a9f8
commit fd7e08a6f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 99 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Include/cpython/pystate.h
View File

@ -309,6 +309,7 @@ PyAPI_FUNC(void) _PyCrossInterpreterData_Clear(
PyAPI_FUNC(int) _PyObject_GetCrossInterpreterData(PyObject *, _PyCrossInterpreterData *);
PyAPI_FUNC(PyObject *) _PyCrossInterpreterData_NewObject(_PyCrossInterpreterData *);
PyAPI_FUNC(int) _PyCrossInterpreterData_Release(_PyCrossInterpreterData *);
PyAPI_FUNC(int) _PyCrossInterpreterData_ReleaseAndRawFree(_PyCrossInterpreterData *);
PyAPI_FUNC(int) _PyObject_CheckCrossInterpreterData(PyObject *);

2
Include/internal/pycore_ceval.h
View File

@ -44,7 +44,7 @@ extern void _PyEval_SignalReceived(PyInterpreterState *interp);
// Export for '_testinternalcapi' shared extension
PyAPI_FUNC(int) _PyEval_AddPendingCall(
PyInterpreterState *interp,
int (*func)(void *),
_Py_pending_call_func func,
void *arg,
int mainthreadonly);

4
Include/internal/pycore_ceval_state.h
View File

@ -11,6 +11,8 @@ extern "C" {
#include "pycore_gil.h" // struct _gil_runtime_state
typedef int (*_Py_pending_call_func)(void *);
struct _pending_calls {
int busy;
PyThread_type_lock lock;
@ -22,7 +24,7 @@ struct _pending_calls {
int async_exc;
#define NPENDINGCALLS 32
struct _pending_call {
int (*func)(void *);
_Py_pending_call_func func;
void *arg;
} calls[NPENDINGCALLS];
int first;

30
Modules/_xxinterpchannelsmodule.c
View File

@ -160,14 +160,24 @@ add_new_type(PyObject *mod, PyType_Spec *spec, crossinterpdatafunc shared)
return cls;
}
#define XID_IGNORE_EXC 1
#define XID_FREE 2
static int
_release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
_release_xid_data(_PyCrossInterpreterData *data, int flags)
{
int ignoreexc = flags & XID_IGNORE_EXC;
PyObject *exc;
if (ignoreexc) {
exc = PyErr_GetRaisedException();
}
int res = _PyCrossInterpreterData_Release(data);
int res;
if (flags & XID_FREE) {
res = _PyCrossInterpreterData_ReleaseAndRawFree(data);
}
else {
res = _PyCrossInterpreterData_Release(data);
}
if (res < 0) {
/* The owning interpreter is already destroyed. */
if (ignoreexc) {
@ -175,6 +185,9 @@ _release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
PyErr_Clear();
}
}
if (flags & XID_FREE) {
/* Either way, we free the data. */
}
if (ignoreexc) {
PyErr_SetRaisedException(exc);
}
@ -366,9 +379,8 @@ static void
_channelitem_clear(_channelitem *item)
{
if (item->data != NULL) {
(void)_release_xid_data(item->data, 1);
// It was allocated in _channel_send().
GLOBAL_FREE(item->data);
(void)_release_xid_data(item->data, XID_IGNORE_EXC & XID_FREE);
item->data = NULL;
}
item->next = NULL;
@ -1439,14 +1451,12 @@ _channel_recv(_channels *channels, int64_t id, PyObject **res)
PyObject *obj = _PyCrossInterpreterData_NewObject(data);
if (obj == NULL) {
assert(PyErr_Occurred());
(void)_release_xid_data(data, 1);
// It was allocated in _channel_send().
GLOBAL_FREE(data);
// It was allocated in _channel_send(), so we free it.
(void)_release_xid_data(data, XID_IGNORE_EXC | XID_FREE);
return -1;
}
int release_res = _release_xid_data(data, 0);
// It was allocated in _channel_send().
GLOBAL_FREE(data);
// It was allocated in _channel_send(), so we free it.
int release_res = _release_xid_data(data, XID_FREE);
if (release_res < 0) {
// The source interpreter has been destroyed already.
assert(PyErr_Occurred());

29
Modules/_xxsubinterpretersmodule.c
View File

@ -58,24 +58,17 @@ add_new_exception(PyObject *mod, const char *name, PyObject *base)
add_new_exception(MOD, MODULE_NAME "." Py_STRINGIFY(NAME), BASE)
static int
_release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
_release_xid_data(_PyCrossInterpreterData *data)
{
PyObject *exc;
if (ignoreexc) {
exc = PyErr_GetRaisedException();
}
PyObject *exc = PyErr_GetRaisedException();
int res = _PyCrossInterpreterData_Release(data);
if (res < 0) {
/* The owning interpreter is already destroyed. */
_PyCrossInterpreterData_Clear(NULL, data);
if (ignoreexc) {
// XXX Emit a warning?
PyErr_Clear();
}
}
if (ignoreexc) {
PyErr_SetRaisedException(exc);
// XXX Emit a warning?
PyErr_Clear();
}
PyErr_SetRaisedException(exc);
return res;
}
@ -145,7 +138,7 @@ _sharednsitem_clear(struct _sharednsitem *item)
PyMem_RawFree((void *)item->name);
item->name = NULL;
}
(void)_release_xid_data(&item->data, 1);
(void)_release_xid_data(&item->data);
}
static int
@ -174,16 +167,16 @@ typedef struct _sharedns {
static _sharedns *
_sharedns_new(Py_ssize_t len)
{
_sharedns *shared = PyMem_NEW(_sharedns, 1);
_sharedns *shared = PyMem_RawCalloc(sizeof(_sharedns), 1);
if (shared == NULL) {
PyErr_NoMemory();
return NULL;
}
shared->len = len;
shared->items = PyMem_NEW(struct _sharednsitem, len);
shared->items = PyMem_RawCalloc(sizeof(struct _sharednsitem), len);
if (shared->items == NULL) {
PyErr_NoMemory();
PyMem_Free(shared);
PyMem_RawFree(shared);
return NULL;
}
return shared;
@ -195,8 +188,8 @@ _sharedns_free(_sharedns *shared)
for (Py_ssize_t i=0; i < shared->len; i++) {
_sharednsitem_clear(&shared->items[i]);
}
PyMem_Free(shared->items);
PyMem_Free(shared);
PyMem_RawFree(shared->items);
PyMem_RawFree(shared);
}
static _sharedns *

8
Python/ceval_gil.c
View File

@ -763,7 +763,7 @@ _PyEval_SignalReceived(PyInterpreterState *interp)
/* Push one item onto the queue while holding the lock. */
static int
_push_pending_call(struct _pending_calls *pending,
int (*func)(void *), void *arg)
_Py_pending_call_func func, void *arg)
{
int i = pending->last;
int j = (i + 1) % NPENDINGCALLS;
@ -810,7 +810,7 @@ _pop_pending_call(struct _pending_calls *pending,
int
_PyEval_AddPendingCall(PyInterpreterState *interp,
int (*func)(void *), void *arg,
_Py_pending_call_func func, void *arg,
int mainthreadonly)
{
assert(!mainthreadonly || _Py_IsMainInterpreter(interp));
@ -834,7 +834,7 @@ _PyEval_AddPendingCall(PyInterpreterState *interp,
}
int
Py_AddPendingCall(int (*func)(void *), void *arg)
Py_AddPendingCall(_Py_pending_call_func func, void *arg)
{
/* Legacy users of this API will continue to target the main thread
(of the main interpreter). */
@ -878,7 +878,7 @@ _make_pending_calls(struct _pending_calls *pending)
{
/* perform a bounded number of calls, in case of recursion */
for (int i=0; i<NPENDINGCALLS; i++) {
int (*func)(void *) = NULL;
_Py_pending_call_func func = NULL;
void *arg = NULL;
/* pop one item off the queue while holding the lock */

93
Python/pystate.c
View File

@ -2309,10 +2309,16 @@ _xidata_init(_PyCrossInterpreterData *data)
static inline void
_xidata_clear(_PyCrossInterpreterData *data)
{
if (data->free != NULL) {
data->free(data->data);
// _PyCrossInterpreterData only has two members that need to be
// cleaned up, if set: "data" must be freed and "obj" must be decref'ed.
// In both cases the original (owning) interpreter must be used,
// which is the caller's responsibility to ensure.
if (data->data != NULL) {
if (data->free != NULL) {
data->free(data->data);
}
data->data = NULL;
}
data->data = NULL;
Py_CLEAR(data->obj);
}
@ -2457,40 +2463,32 @@ _PyCrossInterpreterData_NewObject(_PyCrossInterpreterData *data)
return data->new_object(data);
}
typedef void (*releasefunc)(PyInterpreterState *, void *);
static void
_call_in_interpreter(PyInterpreterState *interp, releasefunc func, void *arg)
static int
_release_xidata_pending(void *data)
{
/* We would use Py_AddPendingCall() if it weren't specific to the
* main interpreter (see bpo-33608). In the meantime we take a
* naive approach.
*/
_PyRuntimeState *runtime = interp->runtime;
PyThreadState *save_tstate = NULL;
if (interp != current_fast_get(runtime)->interp) {
// XXX Using the "head" thread isn't strictly correct.
PyThreadState *tstate = PyInterpreterState_ThreadHead(interp);
// XXX Possible GILState issues?
save_tstate = _PyThreadState_Swap(runtime, tstate);
}
// XXX Once the GIL is per-interpreter, this should be called with the
// calling interpreter's GIL released and the target interpreter's held.
func(interp, arg);
// Switch back.
if (save_tstate != NULL) {
_PyThreadState_Swap(runtime, save_tstate);
}
_xidata_clear((_PyCrossInterpreterData *)data);
return 0;
}
int
_PyCrossInterpreterData_Release(_PyCrossInterpreterData *data)
static int
_xidata_release_and_rawfree_pending(void *data)
{
if (data->free == NULL && data->obj == NULL) {
_xidata_clear((_PyCrossInterpreterData *)data);
PyMem_RawFree(data);
return 0;
}
static int
_xidata_release(_PyCrossInterpreterData *data, int rawfree)
{
if ((data->data == NULL || data->free == NULL) && data->obj == NULL) {
// Nothing to release!
data->data = NULL;
if (rawfree) {
PyMem_RawFree(data);
}
else {
data->data = NULL;
}
return 0;
}
@ -2501,15 +2499,42 @@ _PyCrossInterpreterData_Release(_PyCrossInterpreterData *data)
// This function shouldn't have been called.
// XXX Someone leaked some memory...
assert(PyErr_Occurred());
if (rawfree) {
PyMem_RawFree(data);
}
return -1;
}
// "Release" the data and/or the object.
_call_in_interpreter(interp,
(releasefunc)_PyCrossInterpreterData_Clear, data);
if (interp == current_fast_get(interp->runtime)->interp) {
_xidata_clear(data);
if (rawfree) {
PyMem_RawFree(data);
}
}
else {
_Py_pending_call_func func = _release_xidata_pending;
if (rawfree) {
func = _xidata_release_and_rawfree_pending;
}
// XXX Emit a warning if this fails?
_PyEval_AddPendingCall(interp, func, data, 0);
}
return 0;
}
int
_PyCrossInterpreterData_Release(_PyCrossInterpreterData *data)
{
return _xidata_release(data, 0);
}
int
_PyCrossInterpreterData_ReleaseAndRawFree(_PyCrossInterpreterData *data)
{
return _xidata_release(data, 1);
}
/* registry of {type -> crossinterpdatafunc} */
/* For now we use a global registry of shareable classes. An