From 85f5a69ae1541271286bb0f0e0303aabf792dd5c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 9 Mar 2020 22:12:04 +0100 Subject: bpo-39877: Refactor take_gil() function (GH-18885) * Remove ceval parameter of take_gil(): get it from tstate. * Move exit_thread_if_finalizing() call inside take_gil(). Replace exit_thread_if_finalizing() with tstate_must_exit(): the caller is now responsible to call PyThread_exit_thread(). * Move is_tstate_valid() assertion inside take_gil(). Remove is_tstate_valid(): inline code into take_gil(). * Move gil_created() assertion inside take_gil(). --- Python/ceval.c | 78 +++++++++--------------------------------------------- Python/ceval_gil.h | 41 +++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 66 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 208fdab..72a8304 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -198,16 +198,6 @@ ensure_tstate_not_null(const char *func, PyThreadState *tstate) } -#ifndef NDEBUG -static int is_tstate_valid(PyThreadState *tstate) -{ - assert(!_PyMem_IsPtrFreed(tstate)); - assert(!_PyMem_IsPtrFreed(tstate->interp)); - return 1; -} -#endif - - int PyEval_ThreadsInitialized(void) { @@ -230,13 +220,15 @@ _PyEval_InitThreads(PyThreadState *tstate) PyThread_init_thread(); create_gil(gil); - take_gil(ceval, tstate); + + take_gil(tstate); struct _pending_calls *pending = &ceval->pending; pending->lock = PyThread_allocate_lock(); if (pending->lock == NULL) { return _PyStatus_NO_MEMORY(); } + return _PyStatus_OK(); } @@ -269,30 +261,6 @@ _PyEval_FiniThreads(struct _ceval_runtime_state *ceval) } } -/* This function is designed to exit daemon threads immediately rather than - taking the GIL if Py_Finalize() has been called. - - The caller must *not* hold the GIL, since this function does not release - the GIL before exiting the thread. - - When this function is called by a daemon thread after Py_Finalize() has been - called, the GIL does no longer exist. - - tstate must be non-NULL. */ -static inline void -exit_thread_if_finalizing(PyThreadState *tstate) -{ - /* bpo-39877: Access _PyRuntime directly rather than using - tstate->interp->runtime to support calls from Python daemon threads. - After Py_Finalize() has been called, tstate can be a dangling pointer: - point to PyThreadState freed memory. */ - _PyRuntimeState *runtime = &_PyRuntime; - PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime); - if (finalizing != NULL && finalizing != tstate) { - PyThread_exit_thread(); - } -} - void _PyEval_Fini(void) { @@ -329,11 +297,7 @@ PyEval_AcquireLock(void) PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime); ensure_tstate_not_null(__func__, tstate); - exit_thread_if_finalizing(tstate); - assert(is_tstate_valid(tstate)); - - struct _ceval_runtime_state *ceval = &runtime->ceval; - take_gil(ceval, tstate); + take_gil(tstate); } void @@ -353,17 +317,10 @@ PyEval_AcquireThread(PyThreadState *tstate) { ensure_tstate_not_null(__func__, tstate); - exit_thread_if_finalizing(tstate); - assert(is_tstate_valid(tstate)); - - _PyRuntimeState *runtime = tstate->interp->runtime; - struct _ceval_runtime_state *ceval = &runtime->ceval; - - /* Check that _PyEval_InitThreads() was called to create the lock */ - assert(gil_created(&ceval->gil)); + take_gil(tstate); - take_gil(ceval, tstate); - if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) { + struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate; + if (_PyThreadState_Swap(gilstate, tstate) != NULL) { Py_FatalError("non-NULL old thread state"); } } @@ -396,7 +353,8 @@ _PyEval_ReInitThreads(_PyRuntimeState *runtime) recreate_gil(&ceval->gil); PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime); ensure_tstate_not_null(__func__, tstate); - take_gil(ceval, tstate); + + take_gil(tstate); struct _pending_calls *pending = &ceval->pending; pending->lock = PyThread_allocate_lock(); @@ -436,16 +394,10 @@ PyEval_RestoreThread(PyThreadState *tstate) { ensure_tstate_not_null(__func__, tstate); - exit_thread_if_finalizing(tstate); - assert(is_tstate_valid(tstate)); + take_gil(tstate); - _PyRuntimeState *runtime = tstate->interp->runtime; - struct _ceval_runtime_state *ceval = &runtime->ceval; - assert(gil_created(&ceval->gil)); - - take_gil(ceval, tstate); - - _PyThreadState_Swap(&runtime->gilstate, tstate); + struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate; + _PyThreadState_Swap(gilstate, tstate); } @@ -805,7 +757,6 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) PyThreadState * const tstate = _PyRuntimeState_GetThreadState(runtime); ensure_tstate_not_null(__func__, tstate); - assert(is_tstate_valid(tstate)); /* when tracing we set things up so that @@ -1294,10 +1245,7 @@ main_loop: /* Other threads may run now */ - /* Check if we should make a quick exit. */ - exit_thread_if_finalizing(tstate); - - take_gil(ceval, tstate); + take_gil(tstate); if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) { Py_FatalError("orphan tstate"); diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index 99d576d..03f04b9 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -180,17 +180,55 @@ drop_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate) #endif } + +/* Check if a Python thread must exit immediately, rather than taking the GIL + if Py_Finalize() has been called. + + When this function is called by a daemon thread after Py_Finalize() has been + called, the GIL does no longer exist. + + tstate must be non-NULL. */ +static inline int +tstate_must_exit(PyThreadState *tstate) +{ + /* bpo-39877: Access _PyRuntime directly rather than using + tstate->interp->runtime to support calls from Python daemon threads. + After Py_Finalize() has been called, tstate can be a dangling pointer: + point to PyThreadState freed memory. */ + _PyRuntimeState *runtime = &_PyRuntime; + PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime); + return (finalizing != NULL && finalizing != tstate); +} + + /* Take the GIL. The function saves errno at entry and restores its value at exit. tstate must be non-NULL. */ static void -take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate) +take_gil(PyThreadState *tstate) { int err = errno; + assert(tstate != NULL); + + /* Check if we should make a quick exit. */ + if (tstate_must_exit(tstate)) { + PyThread_exit_thread(); + } + + /* Ensure that tstate is valid: sanity check for PyEval_AcquireThread() and + PyEval_RestoreThread(). Detect if tstate memory was freed. */ + assert(!_PyMem_IsPtrFreed(tstate)); + assert(!_PyMem_IsPtrFreed(tstate->interp)); + + struct _ceval_runtime_state *ceval = &tstate->interp->runtime->ceval; struct _gil_runtime_state *gil = &ceval->gil; + + /* Check that _PyEval_InitThreads() was called to create the lock */ + assert(gil_created(gil)); + MUTEX_LOCK(gil->mutex); if (!_Py_atomic_load_relaxed(&gil->locked)) { @@ -215,6 +253,7 @@ take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate) SET_GIL_DROP_REQUEST(ceval); } } + _ready: #ifdef FORCE_SWITCHING /* This mutex must be taken before modifying gil->last_holder: -- cgit v0.12