From 9ac0b252ab77694a216c27a5622a2dd1359aa3f5 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sun, 8 Mar 2020 17:48:53 +0100 Subject: [PATCH 1/2] bpo-39877: take_gil() now exits the thread if finalizing take_gil() is now responsible to exit the thread if Python is finalizing. Not only exit before trying to acquire the GIL, but exit also once the GIL is succesfully acquired. --- Python/ceval.c | 66 +++++----------------------------------------- Python/ceval_gil.h | 54 ++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 60 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 42f08c4534c643..3f24b91d1fc2a4 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -197,16 +197,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) { @@ -228,7 +218,8 @@ PyEval_InitThreads(void) create_gil(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(); @@ -255,29 +246,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) @@ -315,11 +283,8 @@ PyEval_AcquireLock(void) PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime); ensure_tstate_not_null(__func__, tstate); - exit_thread_if_finalizing(tstate); - assert(is_tstate_valid(tstate)); + take_gil(tstate); - struct _ceval_runtime_state *ceval = &runtime->ceval; - take_gil(ceval, tstate); } void @@ -339,16 +304,9 @@ PyEval_AcquireThread(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; - - /* Check someone has called PyEval_InitThreads() to create the lock */ - assert(gil_created(&ceval->gil)); - - take_gil(ceval, tstate); if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) { Py_FatalError("non-NULL old thread state"); } @@ -382,7 +340,7 @@ _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(); @@ -422,15 +380,9 @@ 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); } @@ -793,7 +745,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 @@ -1282,10 +1233,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) /* 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 99d576d5615171..35d1a08bad9206 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -180,17 +180,57 @@ drop_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate) #endif } + +static inline int +thread_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. + Exit immediately the thread if Py_Finalize() has been called and tstate is + not the thread which called Py_Finalize(). For example, exit daemon threads + which survive after Py_Finalize(). + tstate must be non-NULL. */ static void -take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate) +take_gil(PyThreadState *tstate) { int err = errno; + if (thread_must_exit(tstate)) { + /* bpo-39877: If Py_Finalize() has been called and tstate is not the + thread which called Py_Finalize(), exit immediately the thread. + + This code path can be reached by a daemon thread after Py_Finalize() + completes. In this case, tstate is a dangling pointer: points to + PyThreadState freed memory. + + When this function is called after Py_Finalize() completed, the GIL + does no longer exist. */ + PyThread_exit_thread(); + } + + /* ensure that tstate is valid */ + 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 someone has called PyEval_InitThreads() to create the lock */ + assert(gil_created(gil)); + MUTEX_LOCK(gil->mutex); if (!_Py_atomic_load_relaxed(&gil->locked)) { @@ -243,6 +283,18 @@ take_gil(struct _ceval_runtime_state *ceval, PyThreadState *tstate) MUTEX_UNLOCK(gil->mutex); + if (thread_must_exit(tstate)) { + /* bpo-36475: If Py_Finalize() has been called and tstate is not the + thread which called Py_Finalize(), exit immediately the thread. + + This code path can be reached by a daemon thread which continues to + run after wait_for_thread_shutdown() and before Py_Finalize() + completes. For example, when _PyImport_Cleanup() executes Python + code. */ + drop_gil(ceval, tstate); + PyThread_exit_thread(); + } + errno = err; } From 4f379aa1ffcd2101eff284ea83b660373701e6cb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 9 Mar 2020 00:17:38 +0100 Subject: [PATCH 2/2] take_gil() exit after COND_TIMED_WAIT() --- Python/ceval_gil.h | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index 35d1a08bad9206..d72ea5ef4bb1d3 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -246,6 +246,20 @@ take_gil(PyThreadState *tstate) unsigned long interval = (gil->interval >= 1 ? gil->interval : 1); COND_TIMED_WAIT(gil->cond, gil->mutex, interval, timed_out); + + if (thread_must_exit(tstate)) { + /* bpo-36475: If Py_Finalize() has been called and tstate is not + the thread which called Py_Finalize(), exit immediately the + thread. + + This code path can be reached by a daemon thread which continues + to run after wait_for_thread_shutdown() and before Py_Finalize() + completes. For example, when _PyImport_Cleanup() executes Python + code. */ + MUTEX_UNLOCK(gil->mutex); + PyThread_exit_thread(); + } + /* If we timed out and no switch occurred in the meantime, it is time to ask the GIL-holding thread to drop it. */ if (timed_out && @@ -283,17 +297,7 @@ take_gil(PyThreadState *tstate) MUTEX_UNLOCK(gil->mutex); - if (thread_must_exit(tstate)) { - /* bpo-36475: If Py_Finalize() has been called and tstate is not the - thread which called Py_Finalize(), exit immediately the thread. - - This code path can be reached by a daemon thread which continues to - run after wait_for_thread_shutdown() and before Py_Finalize() - completes. For example, when _PyImport_Cleanup() executes Python - code. */ - drop_gil(ceval, tstate); - PyThread_exit_thread(); - } + assert(!thread_must_exit(tstate)); errno = err; }