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.

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):
 
diff --git a/Misc/NEWS b/Misc/NEWS
index 84f36b8..8be054f 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -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);