diff options
| -rw-r--r-- | Include/frameobject.h | 1 | ||||
| -rw-r--r-- | Lib/test/test_sys.py | 2 | ||||
| -rw-r--r-- | Lib/test/test_threading.py | 38 | ||||
| -rw-r--r-- | Misc/NEWS | 6 | ||||
| -rw-r--r-- | Modules/_testcapimodule.c | 89 | ||||
| -rw-r--r-- | Objects/frameobject.c | 1 | ||||
| -rw-r--r-- | Python/ceval.c | 79 | ||||
| -rw-r--r-- | Python/sysmodule.c | 8 |
8 files changed, 179 insertions, 45 deletions
diff --git a/Include/frameobject.h b/Include/frameobject.h index b41bea6..966ff1f 100644 --- a/Include/frameobject.h +++ b/Include/frameobject.h @@ -39,7 +39,6 @@ typedef struct _frame { /* Borrowed reference to a generator, or NULL */ PyObject *f_gen; - PyThreadState *f_tstate; int f_lasti; /* Last instruction if called */ /* Call PyFrame_GetLineNumber() instead of reading this field directly. As of 2.3 f_lineno is only valid when tracing is diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 9b11f58..177b92e 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -818,7 +818,7 @@ class SizeofTest(unittest.TestCase): nfrees = len(x.f_code.co_freevars) extras = x.f_code.co_stacksize + x.f_code.co_nlocals +\ ncells + nfrees - 1 - check(x, vsize('13P3ic' + CO_MAXBLOCKS*'3i' + 'P' + extras*'P')) + check(x, vsize('12P3ic' + CO_MAXBLOCKS*'3i' + 'P' + extras*'P')) # function def func(): pass check(func, size('12P')) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index a84577c..73839e7 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -662,6 +662,44 @@ class ThreadTests(BaseTestCase): self.assertRegex(err.rstrip(), b"^sys:1: ResourceWarning: unclosed file ") + 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 ThreadJoinOnShutdown(BaseTestCase): @@ -10,6 +10,12 @@ Release date: 2014-01-05 Core and Builtins ----------------- +- Issue #14432: Remove the thread state field from the frame structure. 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 #19576: PyGILState_Ensure() now initializes threads. At startup, Python has no concrete GIL. If PyGILState_Ensure() is called from a new thread for the first time and PyEval_InitThreads() was not called yet, a GIL needs to be diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9662eb8..f01c2b8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2869,6 +2869,93 @@ PyDoc_STRVAR(docstring_with_signature_and_extra_newlines, "This docstring has a valid signature and some extra newlines." ); +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}, {"raise_memoryerror", (PyCFunction)raise_memoryerror, METH_NOARGS}, @@ -2997,6 +3084,8 @@ static PyMethodDef TestMethods[] = { {"docstring_with_signature_and_extra_newlines", (PyCFunction)test_with_docstring, METH_NOARGS, docstring_with_signature_and_extra_newlines}, + {"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/frameobject.c b/Objects/frameobject.c index 63f03a6..0d62293 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -726,7 +726,6 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code, PyObject *globals, Py_INCREF(locals); f->f_locals = locals; } - f->f_tstate = tstate; f->f_lasti = -1; f->f_lineno = code->co_firstlineno; diff --git a/Python/ceval.c b/Python/ceval.c index 82f0651..34c52b0 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -123,13 +123,16 @@ static PyObject * load_args(PyObject ***, int); static int lltrace; static int prtrace(PyObject *, char *); #endif -static int call_trace(Py_tracefunc, PyObject *, PyFrameObject *, +static int call_trace(Py_tracefunc, PyObject *, + PyThreadState *, PyFrameObject *, int, PyObject *); static int call_trace_protected(Py_tracefunc, PyObject *, - PyFrameObject *, int, PyObject *); -static void call_exc_trace(Py_tracefunc, PyObject *, PyFrameObject *); + PyThreadState *, PyFrameObject *, + int, PyObject *); +static void call_exc_trace(Py_tracefunc, PyObject *, + PyThreadState *, PyFrameObject *); static int maybe_call_line_trace(Py_tracefunc, PyObject *, - PyFrameObject *, int *, int *, int *); + PyThreadState *, PyFrameObject *, int *, int *, int *); static PyObject * cmp_outcome(int, PyObject *, PyObject *); static PyObject * import_from(PyObject *, PyObject *); @@ -1136,7 +1139,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) whenever an exception is detected. */ if (call_trace_protected(tstate->c_tracefunc, tstate->c_traceobj, - f, PyTrace_CALL, Py_None)) { + tstate, f, PyTrace_CALL, Py_None)) { /* Trace function raised an error */ goto exit_eval_frame; } @@ -1146,7 +1149,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) return itself and isn't called for "line" events */ if (call_trace_protected(tstate->c_profilefunc, tstate->c_profileobj, - f, PyTrace_CALL, Py_None)) { + tstate, f, PyTrace_CALL, Py_None)) { /* Profile function raised an error */ goto exit_eval_frame; } @@ -1293,8 +1296,8 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) err = maybe_call_line_trace(tstate->c_tracefunc, tstate->c_traceobj, - f, &instr_lb, &instr_ub, - &instr_prev); + tstate, f, + &instr_lb, &instr_ub, &instr_prev); /* Reload possibly changed frame fields */ JUMPTO(f->f_lasti); if (f->f_stacktop != NULL) { @@ -1906,7 +1909,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) PyObject *val; if (tstate->c_tracefunc != NULL && PyErr_ExceptionMatches(PyExc_StopIteration)) - call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, f); + call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, tstate, f); err = _PyGen_FetchStopIterationValue(&val); if (err < 0) goto error; @@ -2658,7 +2661,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) if (!PyErr_ExceptionMatches(PyExc_StopIteration)) goto error; else if (tstate->c_tracefunc != NULL) - call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, f); + call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, tstate, f); PyErr_Clear(); } /* iterator ended normally */ @@ -3054,7 +3057,8 @@ error: PyTraceBack_Here(f); if (tstate->c_tracefunc != NULL) - call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, f); + call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, + tstate, f); fast_block_end: assert(why != WHY_NOT); @@ -3182,8 +3186,8 @@ fast_yield: if (tstate->use_tracing) { if (tstate->c_tracefunc) { if (why == WHY_RETURN || why == WHY_YIELD) { - if (call_trace(tstate->c_tracefunc, - tstate->c_traceobj, f, + if (call_trace(tstate->c_tracefunc, tstate->c_traceobj, + tstate, f, PyTrace_RETURN, retval)) { Py_XDECREF(retval); retval = NULL; @@ -3191,18 +3195,19 @@ fast_yield: } } else if (why == WHY_EXCEPTION) { - call_trace_protected(tstate->c_tracefunc, - tstate->c_traceobj, f, + call_trace_protected(tstate->c_tracefunc, tstate->c_traceobj, + tstate, f, PyTrace_RETURN, NULL); } } if (tstate->c_profilefunc) { if (why == WHY_EXCEPTION) call_trace_protected(tstate->c_profilefunc, - tstate->c_profileobj, f, + tstate->c_profileobj, + tstate, f, PyTrace_RETURN, NULL); - else if (call_trace(tstate->c_profilefunc, - tstate->c_profileobj, f, + else if (call_trace(tstate->c_profilefunc, tstate->c_profileobj, + tstate, f, PyTrace_RETURN, retval)) { Py_XDECREF(retval); retval = NULL; @@ -3846,7 +3851,8 @@ prtrace(PyObject *v, char *str) #endif static void -call_exc_trace(Py_tracefunc func, PyObject *self, PyFrameObject *f) +call_exc_trace(Py_tracefunc func, PyObject *self, + PyThreadState *tstate, PyFrameObject *f) { PyObject *type, *value, *traceback, *orig_traceback, *arg; int err; @@ -3862,7 +3868,7 @@ call_exc_trace(Py_tracefunc func, PyObject *self, PyFrameObject *f) PyErr_Restore(type, value, orig_traceback); return; } - err = call_trace(func, self, f, PyTrace_EXCEPTION, arg); + err = call_trace(func, self, tstate, f, PyTrace_EXCEPTION, arg); Py_DECREF(arg); if (err == 0) PyErr_Restore(type, value, orig_traceback); @@ -3874,13 +3880,14 @@ call_exc_trace(Py_tracefunc func, PyObject *self, PyFrameObject *f) } static int -call_trace_protected(Py_tracefunc func, PyObject *obj, PyFrameObject *frame, +call_trace_protected(Py_tracefunc func, PyObject *obj, + PyThreadState *tstate, PyFrameObject *frame, int what, PyObject *arg) { PyObject *type, *value, *traceback; int err; PyErr_Fetch(&type, &value, &traceback); - err = call_trace(func, obj, frame, what, arg); + err = call_trace(func, obj, tstate, frame, what, arg); if (err == 0) { PyErr_Restore(type, value, traceback); @@ -3895,10 +3902,10 @@ call_trace_protected(Py_tracefunc func, PyObject *obj, PyFrameObject *frame, } static int -call_trace(Py_tracefunc func, PyObject *obj, PyFrameObject *frame, +call_trace(Py_tracefunc func, PyObject *obj, + PyThreadState *tstate, PyFrameObject *frame, int what, PyObject *arg) { - PyThreadState *tstate = frame->f_tstate; int result; if (tstate->tracing) return 0; @@ -3914,8 +3921,7 @@ call_trace(Py_tracefunc func, PyObject *obj, PyFrameObject *frame, PyObject * _PyEval_CallTracing(PyObject *func, PyObject *args) { - PyFrameObject *frame = PyEval_GetFrame(); - PyThreadState *tstate = frame->f_tstate; + PyThreadState *tstate = PyThreadState_GET(); int save_tracing = tstate->tracing; int save_use_tracing = tstate->use_tracing; PyObject *result; @@ -3932,8 +3938,8 @@ _PyEval_CallTracing(PyObject *func, PyObject *args) /* See Objects/lnotab_notes.txt for a description of how tracing works. */ static int maybe_call_line_trace(Py_tracefunc func, PyObject *obj, - PyFrameObject *frame, int *instr_lb, int *instr_ub, - int *instr_prev) + PyThreadState *tstate, PyFrameObject *frame, + int *instr_lb, int *instr_ub, int *instr_prev) { int result = 0; int line = frame->f_lineno; @@ -3953,7 +3959,7 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj, number and call the trace function. */ if (frame->f_lasti == *instr_lb || frame->f_lasti < *instr_prev) { frame->f_lineno = line; - result = call_trace(func, obj, frame, PyTrace_LINE, Py_None); + result = call_trace(func, obj, tstate, frame, PyTrace_LINE, Py_None); } *instr_prev = frame->f_lasti; return result; @@ -4149,10 +4155,9 @@ err_args(PyObject *func, int flags, int nargs) #define C_TRACE(x, call) \ if (tstate->use_tracing && tstate->c_profilefunc) { \ - if (call_trace(tstate->c_profilefunc, \ - tstate->c_profileobj, \ - tstate->frame, PyTrace_C_CALL, \ - func)) { \ + if (call_trace(tstate->c_profilefunc, tstate->c_profileobj, \ + tstate, tstate->frame, \ + PyTrace_C_CALL, func)) { \ x = NULL; \ } \ else { \ @@ -4161,14 +4166,14 @@ if (tstate->use_tracing && tstate->c_profilefunc) { \ if (x == NULL) { \ call_trace_protected(tstate->c_profilefunc, \ tstate->c_profileobj, \ - tstate->frame, PyTrace_C_EXCEPTION, \ - func); \ + tstate, tstate->frame, \ + PyTrace_C_EXCEPTION, func); \ /* XXX should pass (type, value, tb) */ \ } else { \ if (call_trace(tstate->c_profilefunc, \ tstate->c_profileobj, \ - tstate->frame, PyTrace_C_RETURN, \ - func)) { \ + tstate, tstate->frame, \ + PyTrace_C_RETURN, func)) { \ Py_DECREF(x); \ x = NULL; \ } \ diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 4028a01..961657e 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -367,7 +367,7 @@ trace_init(void) static PyObject * -call_trampoline(PyThreadState *tstate, PyObject* callback, +call_trampoline(PyObject* callback, PyFrameObject *frame, int what, PyObject *arg) { PyObject *args; @@ -405,12 +405,11 @@ static int profile_trampoline(PyObject *self, PyFrameObject *frame, int what, PyObject *arg) { - PyThreadState *tstate = frame->f_tstate; PyObject *result; if (arg == NULL) arg = Py_None; - result = call_trampoline(tstate, self, frame, what, arg); + result = call_trampoline(self, frame, what, arg); if (result == NULL) { PyEval_SetProfile(NULL, NULL); return -1; @@ -423,7 +422,6 @@ static int trace_trampoline(PyObject *self, PyFrameObject *frame, int what, PyObject *arg) { - PyThreadState *tstate = frame->f_tstate; PyObject *callback; PyObject *result; @@ -433,7 +431,7 @@ trace_trampoline(PyObject *self, PyFrameObject *frame, callback = frame->f_trace; if (callback == NULL) return 0; - result = call_trampoline(tstate, callback, frame, what, arg); + result = call_trampoline(callback, frame, what, arg); if (result == NULL) { PyEval_SetTrace(NULL, NULL); Py_XDECREF(frame->f_trace); |