From 23ef89db7ae46d160650263cc80479c2ed6693fb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 18 Mar 2020 02:26:04 +0100 Subject: bpo-39984: _PyThreadState_DeleteCurrent() takes tstate (GH-19051) * _PyThreadState_DeleteCurrent() now takes tstate rather than runtime. * Add ensure_tstate_not_null() helper to pystate.c. * Add _PyEval_ReleaseLock() function. * _PyThreadState_DeleteCurrent() now calls _PyEval_ReleaseLock(tstate) and frees PyThreadState memory after this call, not before. * PyGILState_Release(): rename "tcur" variable to "tstate". --- Include/internal/pycore_ceval.h | 2 ++ Include/internal/pycore_pylifecycle.h | 2 +- Lib/test/test_capi.py | 3 +- Modules/_threadmodule.c | 2 +- Python/ceval.c | 10 +++++- Python/pystate.c | 60 +++++++++++++++++++---------------- 6 files changed, 47 insertions(+), 32 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index ec00d18..5aeef6c 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -57,6 +57,8 @@ extern PyObject *_PyEval_EvalCode( extern int _PyEval_ThreadsInitialized(_PyRuntimeState *runtime); extern PyStatus _PyEval_InitThreads(PyThreadState *tstate); +extern void _PyEval_ReleaseLock(PyThreadState *tstate); + /* --- _Py_EnterRecursiveCall() ----------------------------------------- */ diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index cf22803..77ea3f2 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -104,7 +104,7 @@ PyAPI_FUNC(void) _PyErr_Print(PyThreadState *tstate); PyAPI_FUNC(void) _PyErr_Display(PyObject *file, PyObject *exception, PyObject *value, PyObject *tb); -PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(struct pyruntimestate *runtime); +PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(PyThreadState *tstate); #ifdef __cplusplus } diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 5a85c48..0243439 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -62,7 +62,8 @@ class CAPITest(unittest.TestCase): # This used to cause an infinite loop. self.assertTrue(err.rstrip().startswith( b'Fatal Python error: ' - b'PyThreadState_Get: no current thread')) + b'PyThreadState_Get: ' + b'current thread state is NULL (released GIL?)')) def test_memoryview_from_NULL_pointer(self): self.assertRaises(ValueError, _testcapi.make_memoryview_from_NULL_pointer) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 19dd704..544bfdc 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1028,7 +1028,7 @@ t_bootstrap(void *boot_raw) PyMem_DEL(boot_raw); tstate->interp->num_threads--; PyThreadState_Clear(tstate); - _PyThreadState_DeleteCurrent(runtime); + _PyThreadState_DeleteCurrent(tstate); PyThread_exit_thread(); } diff --git a/Python/ceval.c b/Python/ceval.c index b055d61..d4c15f3 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -193,7 +193,8 @@ static void ensure_tstate_not_null(const char *func, PyThreadState *tstate) { if (tstate == NULL) { - _Py_FatalErrorFunc(func, "current thread state is NULL"); + _Py_FatalErrorFunc(func, + "current thread state is NULL (released GIL?)"); } } @@ -314,6 +315,13 @@ PyEval_ReleaseLock(void) } void +_PyEval_ReleaseLock(PyThreadState *tstate) +{ + struct _ceval_runtime_state *ceval = &tstate->interp->runtime->ceval; + drop_gil(ceval, tstate); +} + +void PyEval_AcquireThread(PyThreadState *tstate) { ensure_tstate_not_null(__func__, tstate); diff --git a/Python/pystate.c b/Python/pystate.c index 77d4cc7..a349cea 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -37,6 +37,16 @@ extern "C" { _Py_atomic_store_relaxed(&(gilstate)->tstate_current, \ (uintptr_t)(value)) +static void +ensure_tstate_not_null(const char *func, PyThreadState *tstate) +{ + if (tstate == NULL) { + _Py_FatalErrorFunc(func, + "current thread state is NULL (released GIL?)"); + } +} + + /* Forward declarations */ static PyThreadState *_PyGILState_GetThisThreadState(struct _gilstate_runtime_state *gilstate); static void _PyThreadState_Delete(PyThreadState *tstate, int check_current); @@ -400,9 +410,7 @@ PyInterpreterState * PyInterpreterState_Get(void) { PyThreadState *tstate = _PyThreadState_GET(); - if (tstate == NULL) { - Py_FatalError("no current thread state"); - } + ensure_tstate_not_null(__func__, tstate); PyInterpreterState *interp = tstate->interp; if (interp == NULL) { Py_FatalError("no current interpreter"); @@ -819,9 +827,7 @@ tstate_delete_common(PyThreadState *tstate, struct _gilstate_runtime_state *gilstate) { _PyRuntimeState *runtime = tstate->interp->runtime; - if (tstate == NULL) { - Py_FatalError("NULL tstate"); - } + ensure_tstate_not_null(__func__, tstate); PyInterpreterState *interp = tstate->interp; if (interp == NULL) { Py_FatalError("NULL interp"); @@ -835,8 +841,6 @@ tstate_delete_common(PyThreadState *tstate, tstate->next->prev = tstate->prev; HEAD_UNLOCK(runtime); - PyMem_RawFree(tstate); - if (gilstate->autoInterpreterState && PyThread_tss_get(&gilstate->autoTSSkey) == tstate) { @@ -855,6 +859,7 @@ _PyThreadState_Delete(PyThreadState *tstate, int check_current) } } tstate_delete_common(tstate, gilstate); + PyMem_RawFree(tstate); } @@ -866,22 +871,22 @@ PyThreadState_Delete(PyThreadState *tstate) void -_PyThreadState_DeleteCurrent(_PyRuntimeState *runtime) +_PyThreadState_DeleteCurrent(PyThreadState *tstate) { - struct _gilstate_runtime_state *gilstate = &runtime->gilstate; - PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate); - if (tstate == NULL) { - Py_FatalError("no current tstate"); - } + ensure_tstate_not_null(__func__, tstate); + struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate; tstate_delete_common(tstate, gilstate); _PyRuntimeGILState_SetThreadState(gilstate, NULL); - PyEval_ReleaseLock(); + _PyEval_ReleaseLock(tstate); + PyMem_RawFree(tstate); } void PyThreadState_DeleteCurrent(void) { - _PyThreadState_DeleteCurrent(&_PyRuntime); + struct _gilstate_runtime_state *gilstate = &_PyRuntime.gilstate; + PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate); + _PyThreadState_DeleteCurrent(tstate); } @@ -938,9 +943,7 @@ PyThreadState * PyThreadState_Get(void) { PyThreadState *tstate = _PyThreadState_GET(); - if (tstate == NULL) { - Py_FatalError("no current thread"); - } + ensure_tstate_not_null(__func__, tstate); return tstate; } @@ -1342,8 +1345,8 @@ void PyGILState_Release(PyGILState_STATE oldstate) { _PyRuntimeState *runtime = &_PyRuntime; - PyThreadState *tcur = PyThread_tss_get(&runtime->gilstate.autoTSSkey); - if (tcur == NULL) { + PyThreadState *tstate = PyThread_tss_get(&runtime->gilstate.autoTSSkey); + if (tstate == NULL) { Py_FatalError("auto-releasing thread-state, " "but no thread-state for this thread"); } @@ -1353,26 +1356,27 @@ PyGILState_Release(PyGILState_STATE oldstate) but while this is very new (April 2003), the extra check by release-only users can't hurt. */ - if (!PyThreadState_IsCurrent(tcur)) { + if (!PyThreadState_IsCurrent(tstate)) { Py_FatalError("This thread state must be current when releasing"); } - assert(PyThreadState_IsCurrent(tcur)); - --tcur->gilstate_counter; - assert(tcur->gilstate_counter >= 0); /* illegal counter value */ + assert(PyThreadState_IsCurrent(tstate)); + --tstate->gilstate_counter; + assert(tstate->gilstate_counter >= 0); /* illegal counter value */ /* If we're going to destroy this thread-state, we must * clear it while the GIL is held, as destructors may run. */ - if (tcur->gilstate_counter == 0) { + if (tstate->gilstate_counter == 0) { /* can't have been locked when we created it */ assert(oldstate == PyGILState_UNLOCKED); - PyThreadState_Clear(tcur); + PyThreadState_Clear(tstate); /* Delete the thread-state. Note this releases the GIL too! * It's vital that the GIL be held here, to avoid shutdown * races; see bugs 225673 and 1061968 (that nasty bug has a * habit of coming back). */ - _PyThreadState_DeleteCurrent(runtime); + assert(_PyRuntimeGILState_GetThreadState(&runtime->gilstate) == tstate); + _PyThreadState_DeleteCurrent(tstate); } /* Release the lock if necessary */ else if (oldstate == PyGILState_UNLOCKED) -- cgit v0.12