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().

4 files changed, 51 insertions, 12 deletions

diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst
index 7ef1122..367c069 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 8df7538..90ff72f 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 0000000..6f09751
--- /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 4e165c3..ccd0427 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 @@ main_loop:
                 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");