https://github.com/python/cpython/commit/e10c9de9d74fd4c26b32e6719d96f04a5be... commit: e10c9de9d74fd4c26b32e6719d96f04a5be6987d branch: 3.6 author: Victor Stinner <victor.stinner@gmail.com> committer: GitHub <noreply@github.com> date: 2017-11-30T23:36:49+01:00 summary: bpo-20891: Fix PyGILState_Ensure() (#4650) (#4655) 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. Enhance also embedded tests, backport from master: * Add test_pre_initialization_api() * Set PYTHONIOENCODING environment variable in test_forced_io_encoding() (cherry picked from commit b4d1e1f7c1af6ae33f0e371576c8bcafedb099db) files: A Misc/NEWS.d/next/C API/2017-11-30-18-13-45.bpo-20891.wBnMdF.rst M Lib/test/test_capi.py M Programs/_testembed.c M Python/pystate.c diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 3a29b69f957..6e4286ed881 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -401,23 +401,30 @@ def setUp(self): def tearDown(self): os.chdir(self.oldcwd) - def run_embedded_interpreter(self, *args): + def run_embedded_interpreter(self, *args, env=None): """Runs a test in the embedded interpreter""" cmd = [self.test_exe] cmd.extend(args) + if env is not None and sys.platform == 'win32': + # Windows requires at least the SYSTEMROOT environment variable to + # start Python. + env = env.copy() + env['SYSTEMROOT'] = os.environ['SYSTEMROOT'] + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - universal_newlines=True) + universal_newlines=True, + env=env) (out, err) = p.communicate() self.assertEqual(p.returncode, 0, "bad returncode %d, stderr is %r" % (p.returncode, err)) return out, err - def test_subinterps(self): + def test_repeated_init_and_subinterpreters(self): # This is just a "don't crash" test - out, err = self.run_embedded_interpreter() + out, err = self.run_embedded_interpreter('repeated_init_and_subinterpreters') if support.verbose: print() print(out) @@ -435,13 +442,14 @@ def _get_default_pipe_encoding(): def test_forced_io_encoding(self): # Checks forced configuration of embedded interpreter IO streams - out, err = self.run_embedded_interpreter("forced_io_encoding") + env = dict(os.environ, PYTHONIOENCODING="utf-8:surrogateescape") + out, err = self.run_embedded_interpreter("forced_io_encoding", env=env) if support.verbose: print() print(out) print(err) - expected_errors = sys.__stdout__.errors - expected_stdin_encoding = sys.__stdin__.encoding + expected_stream_encoding = "utf-8" + expected_errors = "surrogateescape" expected_pipe_encoding = self._get_default_pipe_encoding() expected_output = '\n'.join([ "--- Use defaults ---", @@ -469,13 +477,33 @@ def test_forced_io_encoding(self): "stdout: latin-1:replace", "stderr: latin-1:backslashreplace"]) expected_output = expected_output.format( - in_encoding=expected_stdin_encoding, - out_encoding=expected_pipe_encoding, + in_encoding=expected_stream_encoding, + out_encoding=expected_stream_encoding, errors=expected_errors) # This is useful if we ever trip over odd platform behaviour self.maxDiff = None self.assertEqual(out.strip(), expected_output) + def test_pre_initialization_api(self): + """ + Checks the few parts of the C-API that work before the runtine + is initialized (via Py_Initialize()). + """ + env = dict(os.environ, PYTHONPATH=os.pathsep.join(sys.path)) + out, err = self.run_embedded_interpreter("pre_initialization_api", env=env) + 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, '') + class SkipitemTest(unittest.TestCase): 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 39683993ea0..b0f9087115b 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1,4 +1,5 @@ #include <Python.h> +#include "pythread.h" #include <stdio.h> /********************************************************* @@ -33,7 +34,7 @@ static void print_subinterp(void) ); } -static void test_repeated_init_and_subinterpreters(void) +static int test_repeated_init_and_subinterpreters(void) { PyThreadState *mainstate, *substate; #ifdef WITH_THREAD @@ -70,6 +71,7 @@ static void test_repeated_init_and_subinterpreters(void) PyEval_RestoreThread(mainstate); Py_Finalize(); } + return 0; } /***************************************************** @@ -103,7 +105,7 @@ static void check_stdio_details(const char *encoding, const char * errors) Py_Finalize(); } -static void test_forced_io_encoding(void) +static int test_forced_io_encoding(void) { /* Check various combinations */ printf("--- Use defaults ---\n"); @@ -122,19 +124,123 @@ static void test_forced_io_encoding(void) printf("Unexpected success calling Py_SetStandardStreamEncoding"); } Py_Finalize(); + return 0; } -/* Different embedding tests */ -int main(int argc, char *argv[]) + +/********************************************************* + * Test parts of the C-API that work before initialization + *********************************************************/ + +static int test_pre_initialization_api(void) { + /* Leading "./" ensures getpath.c can still find the standard library */ + wchar_t *program = Py_DecodeLocale("./spam", NULL); + if (program == NULL) { + fprintf(stderr, "Fatal error: cannot decode program name\n"); + return 1; + } + Py_SetProgramName(program); - /* TODO: Check the argument string to allow for more test cases */ - if (argc > 1) { - /* For now: assume "forced_io_encoding */ - test_forced_io_encoding(); - } else { - /* Run the original embedding test case by default */ - test_repeated_init_and_subinterpreters(); + Py_Initialize(); + Py_Finalize(); + + PyMem_RawFree(program); + 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(); + + long thrd = PyThread_start_new_thread(bpo20891_thread, &lock); + if (thrd == -1) { + 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. + * + * Names are compared case-sensitively with the first + * argument. If no match is found, or no first argument was + * provided, the names of all test cases are printed and + * the exit code will be -1. + * + * The int returned from test functions is used as the exit + * code, and test_capi treats all non-zero exit codes as a + * failed test. + *********************************************************/ +struct TestCase +{ + const char *name; + int (*func)(void); +}; + +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 } +}; + +int main(int argc, char *argv[]) +{ + if (argc > 1) { + for (struct TestCase *tc = TestCases; tc && tc->name; tc++) { + if (strcmp(argv[1], tc->name) == 0) + return (*tc->func)(); + } + } + + /* No match found, or no test name provided, so display usage */ + printf("Python " PY_VERSION " _testembed executable for embedded interpreter tests\n" + "Normally executed via 'EmbeddingTests' in Lib/test/test_capi.py\n\n" + "Usage: %s TESTNAME\n\nAll available tests:\n", argv[0]); + for (struct TestCase *tc = TestCases; tc && tc->name; tc++) { + printf(" %s\n", tc->name); + } + + /* Non-zero exit code will cause test_capi.py tests to fail. + This is intentional. */ + return -1; +} diff --git a/Python/pystate.c b/Python/pystate.c index 65f9b7ea05d..c0e088055a6 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -865,6 +865,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 @@ -873,10 +875,7 @@ PyGILState_Ensure(void) assert(autoInterpreterState); /* Py_Initialize() hasn't been called! */ tcur = (PyThreadState *)PyThread_get_key_value(autoTLSkey); 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(autoInterpreterState); @@ -887,16 +886,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; }