bpo-38733: PyErr_Occurred() caller must hold the GIL (GH-17080)

vstinner · web-flow · commit d12d0e7c0fe2 · 2019-11-07T12:42:07.000+01:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
 }

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,8 @@ extern "C" {`
`10`	`10`
`11`	`11`	`static inline PyObject* _PyErr_Occurred(PyThreadState *tstate)`
`12`	`12`	`{`
`13`		`- return tstate == NULL ? NULL : tstate->curexc_type;`
	`13`	`+ assert(tstate != NULL);`
	`14`	`+ return tstate->curexc_type;`
`14`	`15`	`}`
`15`	`16`
`16`	`17`
Original file line number	Diff line number	Diff line change
`@@ -2313,12 +2313,13 @@ _PyMem_DebugRawRealloc(void ctx, void p, size_t nbytes)`
`2313`	`2313`	`return data;`
`2314`	`2314`	`}`
`2315`	`2315`
`2316`		`-static void`
	`2316`	`+static inline void`
`2317`	`2317`	`_PyMem_DebugCheckGIL(void)`
`2318`	`2318`	`{`
`2319`		`- if (!PyGILState_Check())`
	`2319`	`+ if (!PyGILState_Check()) {`
`2320`	`2320`	`Py_FatalError("Python memory allocator called "`
`2321`	`2321`	`"without holding the GIL");`
	`2322`	`+ }`
`2322`	`2323`	`}`
`2323`	`2324`
`2324`	`2325`	`static void *`
Original file line number	Diff line number	Diff line change
`@@ -218,6 +218,9 @@ PyErr_SetString(PyObject exception, const char string)`
`218`	`218`	`PyObject* _Py_HOT_FUNCTION`
`219`	`219`	`PyErr_Occurred(void)`
`220`	`220`	`{`
	`221`	`+ /* The caller must hold the GIL. */`
	`222`	`+ assert(PyGILState_Check());`
	`223`	`+`
`221`	`224`	`PyThreadState *tstate = _PyThreadState_GET();`
`222`	`225`	`return _PyErr_Occurred(tstate);`
`223`	`226`	`}`