[Python-checkins] cpython: Issue #14432: Remove the thread state field from the frame structure. Fix a
victor.stinner
python-checkins at python.org
Fri Dec 13 02:08:43 CET 2013
http://hg.python.org/cpython/rev/d2560fd8a008
changeset: 87917:d2560fd8a008
user: Victor Stinner <victor.stinner at gmail.com>
date: Fri Dec 13 02:01:38 2013 +0100
summary:
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.
files:
Include/frameobject.h | 1 -
Lib/test/test_sys.py | 2 +-
Lib/test/test_threading.py | 38 +++++++++++
Misc/NEWS | 6 +
Modules/_testcapimodule.c | 89 ++++++++++++++++++++++++++
Objects/frameobject.c | 1 -
Python/ceval.c | 79 ++++++++++++----------
Python/sysmodule.c | 8 +-
8 files changed, 179 insertions(+), 45 deletions(-)
diff --git a/Include/frameobject.h b/Include/frameobject.h
--- a/Include/frameobject.h
+++ b/Include/frameobject.h
@@ -39,7 +39,6 @@
/* 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
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -818,7 +818,7 @@
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
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -662,6 +662,44 @@
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):
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,12 @@
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
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -2869,6 +2869,93 @@
"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 @@
{"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
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -726,7 +726,6 @@
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
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -123,13 +123,16 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
}
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 @@
#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 @@
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 @@
}
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 @@
}
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 @@
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 @@
/* 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 @@
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 @@
#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 (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
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -367,7 +367,7 @@
static PyObject *
-call_trampoline(PyThreadState *tstate, PyObject* callback,
+call_trampoline(PyObject* callback,
PyFrameObject *frame, int what, PyObject *arg)
{
PyObject *args;
@@ -405,12 +405,11 @@
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 @@
trace_trampoline(PyObject *self, PyFrameObject *frame,
int what, PyObject *arg)
{
- PyThreadState *tstate = frame->f_tstate;
PyObject *callback;
PyObject *result;
@@ -433,7 +431,7 @@
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);
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list