[Python-checkins] bpo-39877: Refactor take_gil() function (GH-18885)

Mon Mar 9 17:12:12 EDT 2020

https://github.com/python/cpython/commit/85f5a69ae1541271286bb0f0e0303aabf792dd5c
commit: 85f5a69ae1541271286bb0f0e0303aabf792dd5c
branch: master
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020年03月09日T22:12:04+01:00
summary:
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().
files:
M Python/ceval.c
M Python/ceval_gil.h

diff --git a/Python/ceval.c b/Python/ceval.c
index 208fdab802c65..72a830468faa6 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 @@ _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 99d576d561517..03f04b93e0a32 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:
