https://github.com/python/cpython/commit/f781d202a2382731b43bade845a58d28a02... commit: f781d202a2382731b43bade845a58d28a02e9ea1 branch: master author: Joannah Nanjekye <33177550+nanjekyejoannah@users.noreply.github.com> committer: Victor Stinner <vstinner@redhat.com> date: 2019-04-29T10:38:45+02:00 summary: bpo-36475: Finalize PyEval_AcquireLock() and PyEval_AcquireThread() properly (GH-12667) PyEval_AcquireLock() and PyEval_AcquireThread() now terminate the current thread if called while the interpreter is finalizing, making them consistent with PyEval_RestoreThread(), Py_END_ALLOW_THREADS, and PyGILState_Ensure(). files: A Misc/NEWS.d/next/Core and Builtins/2019-04-02-20-02-22.bpo-36475.CjRps3.rst M Doc/c-api/init.rst M Doc/whatsnew/3.8.rst M Python/ceval.c diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 7ef11228a33d..367c069a7ff4 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1080,6 +1080,18 @@ All of the following functions must be called after :c:func:`Py_Initialize`. *tstate*, which should not be *NULL*. The lock must have been created earlier. If this thread already has the lock, deadlock ensues. + .. note:: + Calling this function from a thread when the runtime is finalizing + will terminate the thread, even if the thread was not created by Python. + You can use :c:func:`_Py_IsFinalizing` or :func:`sys.is_finalizing` to + check if the interpreter is in process of being finalized before calling + this function to avoid unwanted termination. + + .. versionchanged:: 3.8 + Updated to be consistent with :c:func:`PyEval_RestoreThread`, + :c:func:`Py_END_ALLOW_THREADS`, and :c:func:`PyGILState_Ensure`, + and terminate the current thread if called while the interpreter is finalizing. + :c:func:`PyEval_RestoreThread` is a higher-level function which is always available (even when threads have not been initialized). @@ -1106,6 +1118,18 @@ All of the following functions must be called after :c:func:`Py_Initialize`. :c:func:`PyEval_RestoreThread` or :c:func:`PyEval_AcquireThread` instead. + .. note:: + Calling this function from a thread when the runtime is finalizing + will terminate the thread, even if the thread was not created by Python. + You can use :c:func:`_Py_IsFinalizing` or :func:`sys.is_finalizing` to + check if the interpreter is in process of being finalized before calling + this function to avoid unwanted termination. + + .. versionchanged:: 3.8 + Updated to be consistent with :c:func:`PyEval_RestoreThread`, + :c:func:`Py_END_ALLOW_THREADS`, and :c:func:`PyGILState_Ensure`, + and terminate the current thread if called while the interpreter is finalizing. + .. c:function:: void PyEval_ReleaseLock() diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 8df7538f1c43..90ff72f67ff4 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -758,6 +758,13 @@ Changes in Python behavior always use the ``sys.platform.startswith('aix')``. (Contributed by M. Felt in :issue:`36588`.) +* :c:func:`PyEval_AcquireLock` and :c:func:`PyEval_AcquireThread` now + terminate the current thread if called while the interpreter is + finalizing, making them consistent with :c:func:`PyEval_RestoreThread`, + :c:func:`Py_END_ALLOW_THREADS`, and :c:func:`PyGILState_Ensure`. If this + behaviour is not desired, guard the call by checking :c:func:`_Py_IsFinalizing` + or :c:func:`sys.is_finalizing`. + Changes in the Python API ------------------------- diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-04-02-20-02-22.bpo-36475.CjRps3.rst b/Misc/NEWS.d/next/Core and Builtins/2019-04-02-20-02-22.bpo-36475.CjRps3.rst new file mode 100644 index 000000000000..6f0975107fa2 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-04-02-20-02-22.bpo-36475.CjRps3.rst @@ -0,0 +1,4 @@ +:c:func:`PyEval_AcquireLock` and :c:func:`PyEval_AcquireThread` now +terminate the current thread if called while the interpreter is +finalizing, making them consistent with :c:func:`PyEval_RestoreThread`, +:c:func:`Py_END_ALLOW_THREADS`, and :c:func:`PyGILState_Ensure`. \ No newline at end of file diff --git a/Python/ceval.c b/Python/ceval.c index 4e165c3311d4..ccd0427a1429 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -76,6 +76,7 @@ static PyObject * special_lookup(PyObject *, _Py_Identifier *); static int check_args_iterable(PyObject *func, PyObject *vararg); static void format_kwargs_error(PyObject *func, PyObject *kwargs); static void format_awaitable_error(PyTypeObject *, int); +static inline void exit_thread_if_finalizing(PyThreadState *); #define NAME_ERROR_MSG \ "name '%.200s' is not defined" @@ -203,6 +204,17 @@ _PyEval_FiniThreads(void) } } +static inline void +exit_thread_if_finalizing(PyThreadState *tstate) +{ + /* _Py_Finalizing is protected by the GIL */ + if (_Py_IsFinalizing() && !_Py_CURRENTLY_FINALIZING(tstate)) { + drop_gil(tstate); + PyThread_exit_thread(); + Py_UNREACHABLE(); + } +} + void PyEval_AcquireLock(void) { @@ -210,6 +222,7 @@ PyEval_AcquireLock(void) if (tstate == NULL) Py_FatalError("PyEval_AcquireLock: current thread state is NULL"); take_gil(tstate); + exit_thread_if_finalizing(tstate); } void @@ -230,6 +243,7 @@ PyEval_AcquireThread(PyThreadState *tstate) /* Check someone has called PyEval_InitThreads() to create the lock */ assert(gil_created()); take_gil(tstate); + exit_thread_if_finalizing(tstate); if (PyThreadState_Swap(tstate) != NULL) Py_FatalError( "PyEval_AcquireThread: non-NULL old thread state"); @@ -298,12 +312,7 @@ PyEval_RestoreThread(PyThreadState *tstate) int err = errno; take_gil(tstate); - /* _Py_Finalizing is protected by the GIL */ - if (_Py_IsFinalizing() && !_Py_CURRENTLY_FINALIZING(tstate)) { - drop_gil(tstate); - PyThread_exit_thread(); - Py_UNREACHABLE(); - } + exit_thread_if_finalizing(tstate); errno = err; PyThreadState_Swap(tstate); @@ -1083,12 +1092,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) take_gil(tstate); /* Check if we should make a quick exit. */ - if (_Py_IsFinalizing() && - !_Py_CURRENTLY_FINALIZING(tstate)) - { - drop_gil(tstate); - PyThread_exit_thread(); - } + exit_thread_if_finalizing(tstate); if (PyThreadState_Swap(tstate) != NULL) Py_FatalError("ceval: orphan tstate");