From ed06c19e02e5f44f14391e768c77c5ae3bcdc906 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 4 Feb 2025 21:27:29 +0000 Subject: [PATCH] gh-129668: Fix thread-safety of MemoryError freelist The MemoryError freelist was not thread-safe in the free threaded build. Use a mutex to protect accesses to the freelist. Unlike other freelists, the MemoryError freelist is not performance sensitive. --- Include/internal/pycore_exceptions.h | 3 + ...-02-04-21-26-05.gh-issue-129668.zDanyM.rst | 2 + Objects/exceptions.c | 62 +++++++++++-------- 3 files changed, 41 insertions(+), 26 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-02-04-21-26-05.gh-issue-129668.zDanyM.rst diff --git a/Include/internal/pycore_exceptions.h b/Include/internal/pycore_exceptions.h index 4a9df709131998..26456d1966bbb0 100644 --- a/Include/internal/pycore_exceptions.h +++ b/Include/internal/pycore_exceptions.h @@ -24,6 +24,9 @@ struct _Py_exc_state { PyObject *errnomap; PyBaseExceptionObject *memerrors_freelist; int memerrors_numfree; +#ifdef Py_GIL_DISABLED + PyMutex memerrors_lock; +#endif // The ExceptionGroup type PyObject *PyExc_ExceptionGroup; }; diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-04-21-26-05.gh-issue-129668.zDanyM.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-04-21-26-05.gh-issue-129668.zDanyM.rst new file mode 100644 index 00000000000000..e42ef57c3164a1 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-04-21-26-05.gh-issue-129668.zDanyM.rst @@ -0,0 +1,2 @@ +Fix race condition when raising :exc:`MemoryError` in the free threaded +build. diff --git a/Objects/exceptions.c b/Objects/exceptions.c index ea2733435fc3ec..154cde9316866e 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -3832,36 +3832,43 @@ SimpleExtendsException(PyExc_Exception, ReferenceError, #define MEMERRORS_SAVE 16 +#ifdef Py_GIL_DISABLED +# define MEMERRORS_LOCK(state) PyMutex_LockFlags(&state->memerrors_lock, _Py_LOCK_DONT_DETACH) +# define MEMERRORS_UNLOCK(state) PyMutex_Unlock(&state->memerrors_lock) +#else +# define MEMERRORS_LOCK(state) ((void)0) +# define MEMERRORS_UNLOCK(state) ((void)0) +#endif + static PyObject * get_memory_error(int allow_allocation, PyObject *args, PyObject *kwds) { - PyBaseExceptionObject *self; + PyBaseExceptionObject *self = NULL; struct _Py_exc_state *state = get_exc_state(); - if (state->memerrors_freelist == NULL) { - if (!allow_allocation) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - return Py_NewRef( - &_Py_INTERP_SINGLETON(interp, last_resort_memory_error)); - } - PyObject *result = BaseException_new((PyTypeObject *)PyExc_MemoryError, args, kwds); - return result; - } - /* Fetch object from freelist and revive it */ - self = state->memerrors_freelist; - self->args = PyTuple_New(0); - /* This shouldn't happen since the empty tuple is persistent */ + MEMERRORS_LOCK(state); + if (state->memerrors_freelist != NULL) { + /* Fetch MemoryError from freelist and initialize it */ + self = state->memerrors_freelist; + state->memerrors_freelist = (PyBaseExceptionObject *) self->dict; + state->memerrors_numfree--; + self->dict = NULL; + self->args = (PyObject *)&_Py_SINGLETON(tuple_empty); + _Py_NewReference((PyObject *)self); + _PyObject_GC_TRACK(self); + } + MEMERRORS_UNLOCK(state); - if (self->args == NULL) { - return NULL; + if (self != NULL) { + return (PyObject *)self; } - state->memerrors_freelist = (PyBaseExceptionObject *) self->dict; - state->memerrors_numfree--; - self->dict = NULL; - _Py_NewReference((PyObject *)self); - _PyObject_GC_TRACK(self); - return (PyObject *)self; + if (!allow_allocation) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + return Py_NewRef( + &_Py_INTERP_SINGLETON(interp, last_resort_memory_error)); + } + return BaseException_new((PyTypeObject *)PyExc_MemoryError, args, kwds); } static PyObject * @@ -3907,14 +3914,17 @@ MemoryError_dealloc(PyObject *obj) } struct _Py_exc_state *state = get_exc_state(); - if (state->memerrors_numfree >= MEMERRORS_SAVE) { - Py_TYPE(self)->tp_free((PyObject *)self); - } - else { + MEMERRORS_LOCK(state); + if (state->memerrors_numfree < MEMERRORS_SAVE) { self->dict = (PyObject *) state->memerrors_freelist; state->memerrors_freelist = self; state->memerrors_numfree++; + MEMERRORS_UNLOCK(state); + return; } + MEMERRORS_UNLOCK(state); + + Py_TYPE(self)->tp_free((PyObject *)self); } static int