From d12d0e7c0fe2b49c40ac4d66365147c619d6c475 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 7 Nov 2019 12:42:07 +0100 Subject: [PATCH] bpo-38733: PyErr_Occurred() caller must hold the GIL (GH-17080) bpo-3605, bpo-38733: Optimize _PyErr_Occurred(): remove "tstate == NULL" test. Py_FatalError() no longer calls PyErr_Occurred() if called without holding the GIL. So PyErr_Occurred() no longer has to support tstate==NULL case. _Py_CheckFunctionResult(): use directly _PyErr_Occurred() to avoid explicit "!= NULL" test. --- Doc/c-api/exceptions.rst | 2 ++ Include/internal/pycore_pyerrors.h | 3 ++- Objects/call.c | 6 ++---- Objects/obmalloc.c | 5 +++-- Python/errors.c | 3 +++ 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst index a042c6eee0a..cd6df00aeb5 100644 --- a/Doc/c-api/exceptions.rst +++ b/Doc/c-api/exceptions.rst @@ -374,6 +374,8 @@ Querying the error indicator own a reference to the return value, so you do not need to :c:func:`Py_DECREF` it. + The caller must hold the GIL. + .. note:: Do not compare the return value to a specific exception; use diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index edbfdfa597e..f3aa3e8b98a 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -10,7 +10,8 @@ extern "C" { static inline PyObject* _PyErr_Occurred(PyThreadState *tstate) { - return tstate == NULL ? NULL : tstate->curexc_type; + assert(tstate != NULL); + return tstate->curexc_type; } diff --git a/Objects/call.c b/Objects/call.c index 0d5c41295c2..a1d0b332cef 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -30,12 +30,10 @@ PyObject* _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable, PyObject *result, const char *where) { - int err_occurred = (_PyErr_Occurred(tstate) != NULL); - assert((callable != NULL) ^ (where != NULL)); if (result == NULL) { - if (!err_occurred) { + if (!_PyErr_Occurred(tstate)) { if (callable) _PyErr_Format(tstate, PyExc_SystemError, "%R returned NULL without setting an error", @@ -52,7 +50,7 @@ _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable, } } else { - if (err_occurred) { + if (_PyErr_Occurred(tstate)) { Py_DECREF(result); if (callable) { diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 50701dbd384..722e91e3db4 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -2313,12 +2313,13 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) return data; } -static void +static inline void _PyMem_DebugCheckGIL(void) { - if (!PyGILState_Check()) + if (!PyGILState_Check()) { Py_FatalError("Python memory allocator called " "without holding the GIL"); + } } static void * diff --git a/Python/errors.c b/Python/errors.c index 9658afeb9f7..1783084c336 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -218,6 +218,9 @@ PyErr_SetString(PyObject *exception, const char *string) PyObject* _Py_HOT_FUNCTION PyErr_Occurred(void) { + /* The caller must hold the GIL. */ + assert(PyGILState_Check()); + PyThreadState *tstate = _PyThreadState_GET(); return _PyErr_Occurred(tstate); }