diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 0ebeb39cbdc..fee48a39102 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -17,6 +17,10 @@ import os from test.script_helper import assert_python_ok, assert_python_failure import subprocess +try: + import _testcapi +except ImportError: + _testcapi = None from test import lock_tests @@ -769,6 +773,45 @@ def do_fork_and_wait(): for t in threads: t.join() + @unittest.skipIf(_testcapi is None, "need _testcapi module") + def test_frame_tstate_tracing(self): + # Issue #14432: Crash when a generator is created in a C thread that is + # destroyed while the generator is still used. The issue was that a + # generator contains a frame, and the frame kept a reference to the + # Python state of the destroyed C thread. The crash occurs when a trace + # function is setup. + + def noop_trace(frame, event, arg): + # no operation + return noop_trace + + def generator(): + while 1: + yield "genereator" + + def callback(): + if callback.gen is None: + callback.gen = generator() + return next(callback.gen) + callback.gen = None + + old_trace = sys.gettrace() + sys.settrace(noop_trace) + try: + # Install a trace function + threading.settrace(noop_trace) + + # Create a generator in a C thread which exits after the call + _testcapi.call_in_temporary_c_thread(callback) + + # Call the generator in a different Python thread, check that the + # generator didn't keep a reference to the destroyed thread state + for test in range(3): + # The trace function is still called here + callback() + finally: + sys.settrace(old_trace) + class ThreadingExceptionTests(BaseTestCase): # A RuntimeError should be raised if Thread.start() is called diff --git a/Misc/NEWS b/Misc/NEWS index 35e7a06a1f1..445a257b218 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,12 @@ What's New in Python 3.3.4 release candidate 1? Core and Builtins ----------------- +- Issue #14432: Generator now clears the borrowed reference to the thread + state. Fix a crash when a generator is created in a C thread that is + destroyed while the generator is still used. The issue was that a generator + contains a frame, and the frame kept a reference to the Python state of the + destroyed C thread. The crash occurs when a trace function is setup. + - Issue #17576: Deprecation warning emitted now when __int__() or __index__() return not int instance. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 7df3e19fc60..057b9e221fd 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2477,6 +2477,93 @@ test_pytime_object_to_timespec(PyObject *self, PyObject *args) return Py_BuildValue("Nl", _PyLong_FromTime_t(sec), nsec); } +typedef struct { + PyThread_type_lock start_event; + PyThread_type_lock exit_event; + PyObject *callback; +} test_c_thread_t; + +static void +temporary_c_thread(void *data) +{ + test_c_thread_t *test_c_thread = data; + PyGILState_STATE state; + PyObject *res; + + PyThread_release_lock(test_c_thread->start_event); + + /* Allocate a Python thread state for this thread */ + state = PyGILState_Ensure(); + + res = PyObject_CallFunction(test_c_thread->callback, "", NULL); + Py_CLEAR(test_c_thread->callback); + + if (res == NULL) { + PyErr_Print(); + } + else { + Py_DECREF(res); + } + + /* Destroy the Python thread state for this thread */ + PyGILState_Release(state); + + PyThread_release_lock(test_c_thread->exit_event); + + PyThread_exit_thread(); +} + +static PyObject * +call_in_temporary_c_thread(PyObject *self, PyObject *callback) +{ + PyObject *res = NULL; + test_c_thread_t test_c_thread; + long thread; + + PyEval_InitThreads(); + + test_c_thread.start_event = PyThread_allocate_lock(); + test_c_thread.exit_event = PyThread_allocate_lock(); + test_c_thread.callback = NULL; + if (!test_c_thread.start_event || !test_c_thread.exit_event) { + PyErr_SetString(PyExc_RuntimeError, "could not allocate lock"); + goto exit; + } + + Py_INCREF(callback); + test_c_thread.callback = callback; + + PyThread_acquire_lock(test_c_thread.start_event, 1); + PyThread_acquire_lock(test_c_thread.exit_event, 1); + + thread = PyThread_start_new_thread(temporary_c_thread, &test_c_thread); + if (thread == -1) { + PyErr_SetString(PyExc_RuntimeError, "unable to start the thread"); + PyThread_release_lock(test_c_thread.start_event); + PyThread_release_lock(test_c_thread.exit_event); + goto exit; + } + + PyThread_acquire_lock(test_c_thread.start_event, 1); + PyThread_release_lock(test_c_thread.start_event); + + Py_BEGIN_ALLOW_THREADS + PyThread_acquire_lock(test_c_thread.exit_event, 1); + PyThread_release_lock(test_c_thread.exit_event); + Py_END_ALLOW_THREADS + + Py_INCREF(Py_None); + res = Py_None; + +exit: + Py_CLEAR(test_c_thread.callback); + if (test_c_thread.start_event) + PyThread_free_lock(test_c_thread.start_event); + if (test_c_thread.exit_event) + PyThread_free_lock(test_c_thread.exit_event); + return res; +} + static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, @@ -2574,6 +2661,8 @@ static PyMethodDef TestMethods[] = { {"pytime_object_to_time_t", test_pytime_object_to_time_t, METH_VARARGS}, {"pytime_object_to_timeval", test_pytime_object_to_timeval, METH_VARARGS}, {"pytime_object_to_timespec", test_pytime_object_to_timespec, METH_VARARGS}, + {"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_O, + PyDoc_STR("set_error_class(error_class) -> None")}, {NULL, NULL} /* sentinel */ }; diff --git a/Objects/genobject.c b/Objects/genobject.c index 016bfa29752..2e74b8c9393 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -76,6 +76,7 @@ gen_send_ex(PyGenObject *gen, PyObject *arg, int exc) /* Generators always return to their most recent caller, not * necessarily their creator. */ + f->f_tstate = tstate; Py_XINCREF(tstate->frame); assert(f->f_back == NULL); f->f_back = tstate->frame; @@ -89,6 +90,8 @@ gen_send_ex(PyGenObject *gen, PyObject *arg, int exc) * cycle. */ assert(f->f_back == tstate->frame); Py_CLEAR(f->f_back); + /* Clear the borrowed reference to the thread state */ + f->f_tstate = NULL; /* If the generator just returned (as opposed to yielding), signal * that the generator is exhausted. */