[Python-checkins] bpo-20891: Fix PyGILState_Ensure() (#4650)
Victor Stinner
webhook-mailer at python.org
Thu Nov 30 16:05:03 EST 2017
https://github.com/python/cpython/commit/b4d1e1f7c1af6ae33f0e371576c8bcafedb099db
commit: b4d1e1f7c1af6ae33f0e371576c8bcafedb099db
branch: master
author: Victor Stinner <victor.stinner at gmail.com>
committer: GitHub <noreply at github.com>
date: 2017-11-30T22:05:00+01:00
summary:
bpo-20891: Fix PyGILState_Ensure() (#4650)
When PyGILState_Ensure() is called in a non-Python thread before
PyEval_InitThreads(), only call PyEval_InitThreads() after calling
PyThreadState_New() to fix a crash.
Add an unit test in test_embed.
files:
A Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst
M Doc/c-api/init.rst
M Lib/test/test_embed.py
M Programs/_testembed.c
M Python/pystate.c
diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst
index 2f77bb359d2..a9927aba5e1 100644
--- a/Doc/c-api/init.rst
+++ b/Doc/c-api/init.rst
@@ -58,8 +58,9 @@ The following functions can be safely called before Python is initialized:
The following functions **should not be called** before
:c:func:`Py_Initialize`: :c:func:`Py_EncodeLocale`, :c:func:`Py_GetPath`,
- :c:func:`Py_GetPrefix`, :c:func:`Py_GetExecPrefix` and
- :c:func:`Py_GetProgramFullPath` and :c:func:`Py_GetPythonHome`.
+ :c:func:`Py_GetPrefix`, :c:func:`Py_GetExecPrefix`,
+ :c:func:`Py_GetProgramFullPath`, :c:func:`Py_GetPythonHome` and
+ :c:func:`PyEval_InitThreads`.
.. _global-conf-vars:
diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
index 8d44543ffd6..c7f45b59acc 100644
--- a/Lib/test/test_embed.py
+++ b/Lib/test/test_embed.py
@@ -198,6 +198,16 @@ def test_pre_initialization_api(self):
self.assertEqual(out, '')
self.assertEqual(err, '')
+ def test_bpo20891(self):
+ """
+ bpo-20891: Calling PyGILState_Ensure in a non-Python thread before
+ calling PyEval_InitThreads() must not crash. PyGILState_Ensure() must
+ call PyEval_InitThreads() for us in this case.
+ """
+ out, err = self.run_embedded_interpreter("bpo20891")
+ self.assertEqual(out, '')
+ self.assertEqual(err, '')
+
if __name__ == "__main__":
unittest.main()
diff --git a/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst
new file mode 100644
index 00000000000..e89cf1292af
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst
@@ -0,0 +1,3 @@
+Fix PyGILState_Ensure(). When PyGILState_Ensure() is called in a non-Python
+thread before PyEval_InitThreads(), only call PyEval_InitThreads() after
+calling PyThreadState_New() to fix a crash.
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
index 21aa76e9de3..a528f3e3aa0 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -1,4 +1,5 @@
#include <Python.h>
+#include "pythread.h"
#include <inttypes.h>
#include <stdio.h>
@@ -147,6 +148,53 @@ static int test_pre_initialization_api(void)
return 0;
}
+static void bpo20891_thread(void *lockp)
+{
+ PyThread_type_lock lock = *((PyThread_type_lock*)lockp);
+
+ PyGILState_STATE state = PyGILState_Ensure();
+ if (!PyGILState_Check()) {
+ fprintf(stderr, "PyGILState_Check failed!");
+ abort();
+ }
+
+ PyGILState_Release(state);
+
+ PyThread_release_lock(lock);
+
+ PyThread_exit_thread();
+}
+
+static int test_bpo20891(void)
+{
+ /* bpo-20891: Calling PyGILState_Ensure in a non-Python thread before
+ calling PyEval_InitThreads() must not crash. PyGILState_Ensure() must
+ call PyEval_InitThreads() for us in this case. */
+ PyThread_type_lock lock = PyThread_allocate_lock();
+ if (!lock) {
+ fprintf(stderr, "PyThread_allocate_lock failed!");
+ return 1;
+ }
+
+ _testembed_Py_Initialize();
+
+ unsigned long thrd = PyThread_start_new_thread(bpo20891_thread, &lock);
+ if (thrd == PYTHREAD_INVALID_THREAD_ID) {
+ fprintf(stderr, "PyThread_start_new_thread failed!");
+ return 1;
+ }
+ PyThread_acquire_lock(lock, WAIT_LOCK);
+
+ Py_BEGIN_ALLOW_THREADS
+ /* wait until the thread exit */
+ PyThread_acquire_lock(lock, WAIT_LOCK);
+ Py_END_ALLOW_THREADS
+
+ PyThread_free_lock(lock);
+
+ return 0;
+}
+
/* *********************************************************
* List of test cases and the function that implements it.
@@ -170,6 +218,7 @@ static struct TestCase TestCases[] = {
{ "forced_io_encoding", test_forced_io_encoding },
{ "repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters },
{ "pre_initialization_api", test_pre_initialization_api },
+ { "bpo20891", test_bpo20891 },
{ NULL, NULL }
};
diff --git a/Python/pystate.c b/Python/pystate.c
index 0fb8ed07195..500f9676875 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -922,6 +922,8 @@ PyGILState_Ensure(void)
{
int current;
PyThreadState *tcur;
+ int need_init_threads = 0;
+
/* Note that we do not auto-init Python here - apart from
potential races with 2 threads auto-initializing, pep-311
spells out other issues. Embedders are expected to have
@@ -929,12 +931,10 @@ PyGILState_Ensure(void)
*/
/* Py_Initialize() hasn't been called! */
assert(_PyRuntime.gilstate.autoInterpreterState);
+
tcur = (PyThreadState *)PyThread_tss_get(&_PyRuntime.gilstate.autoTSSkey);
if (tcur == NULL) {
- /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is
- called from a new thread for the first time, we need the create the
- GIL. */
- PyEval_InitThreads();
+ need_init_threads = 1;
/* Create a new thread state for this thread */
tcur = PyThreadState_New(_PyRuntime.gilstate.autoInterpreterState);
@@ -945,16 +945,28 @@ PyGILState_Ensure(void)
tcur->gilstate_counter = 0;
current = 0; /* new thread state is never current */
}
- else
+ else {
current = PyThreadState_IsCurrent(tcur);
- if (current == 0)
+ }
+
+ if (current == 0) {
PyEval_RestoreThread(tcur);
+ }
+
/* Update our counter in the thread-state - no need for locks:
- tcur will remain valid as we hold the GIL.
- the counter is safe as we are the only thread "allowed"
to modify this value
*/
++tcur->gilstate_counter;
+
+ if (need_init_threads) {
+ /* At startup, Python has no concrete GIL. If PyGILState_Ensure() is
+ called from a new thread for the first time, we need the create the
+ GIL. */
+ PyEval_InitThreads();
+ }
+
return current ? PyGILState_LOCKED : PyGILState_UNLOCKED;
}
More information about the Python-checkins
mailing list