Skip to content

Commit fdeb6ec

Browse files
vstinnervstinner
committed
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.
1 parent 62ca100 commit fdeb6ec

File tree

8 files changed

+179
-45
lines changed

8 files changed

+179
-45
lines changed

‎Include/frameobject.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ typedef struct _frame {
3939
/* Borrowed reference to a generator, or NULL */
4040
PyObject *f_gen;
4141

42-
PyThreadState *f_tstate;
4342
int f_lasti; /* Last instruction if called */
4443
/* Call PyFrame_GetLineNumber() instead of reading this field
4544
directly. As of 2.3 f_lineno is only valid when tracing is

‎Lib/test/test_sys.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,7 @@ class C(object): pass
818818
nfrees = len(x.f_code.co_freevars)
819819
extras = x.f_code.co_stacksize + x.f_code.co_nlocals +\
820820
ncells + nfrees - 1
821-
check(x, vsize('13P3ic' + CO_MAXBLOCKS*'3i' + 'P' + extras*'P'))
821+
check(x, vsize('12P3ic' + CO_MAXBLOCKS*'3i' + 'P' + extras*'P'))
822822
# function
823823
def func(): pass
824824
check(func, size('12P'))

‎Lib/test/test_threading.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,44 @@ def open_sleep():
662662
self.assertRegex(err.rstrip(),
663663
b"^sys:1: ResourceWarning: unclosed file ")
664664

665+
def test_frame_tstate_tracing(self):
666+
# Issue #14432: Crash when a generator is created in a C thread that is
667+
# destroyed while the generator is still used. The issue was that a
668+
# generator contains a frame, and the frame kept a reference to the
669+
# Python state of the destroyed C thread. The crash occurs when a trace
670+
# function is setup.
671+
672+
def noop_trace(frame, event, arg):
673+
# no operation
674+
return noop_trace
675+
676+
def generator():
677+
while 1:
678+
yield "genereator"
679+
680+
def callback():
681+
if callback.gen is None:
682+
callback.gen = generator()
683+
return next(callback.gen)
684+
callback.gen = None
685+
686+
old_trace = sys.gettrace()
687+
sys.settrace(noop_trace)
688+
try:
689+
# Install a trace function
690+
threading.settrace(noop_trace)
691+
692+
# Create a generator in a C thread which exits after the call
693+
_testcapi.call_in_temporary_c_thread(callback)
694+
695+
# Call the generator in a different Python thread, check that the
696+
# generator didn't keep a reference to the destroyed thread state
697+
for test in range(3):
698+
# The trace function is still called here
699+
callback()
700+
finally:
701+
sys.settrace(old_trace)
702+
665703

666704
class ThreadJoinOnShutdown(BaseTestCase):
667705

‎Misc/NEWS‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ Release date: 2014-01-05
1010
Core and Builtins
1111
-----------------
1212

13+
- Issue #14432: Remove the thread state field from the frame structure. Fix a
14+
crash when a generator is created in a C thread that is destroyed while the
15+
generator is still used. The issue was that a generator contains a frame, and
16+
the frame kept a reference to the Python state of the destroyed C thread. The
17+
crash occurs when a trace function is setup.
18+
1319
- Issue #19576: PyGILState_Ensure() now initializes threads. At startup, Python
1420
has no concrete GIL. If PyGILState_Ensure() is called from a new thread for
1521
the first time and PyEval_InitThreads() was not called yet, a GIL needs to be

‎Modules/_testcapimodule.c‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2869,6 +2869,93 @@ PyDoc_STRVAR(docstring_with_signature_and_extra_newlines,
28692869
"This docstring has a valid signature and some extra newlines."
28702870
);
28712871

2872+
typedef struct {
2873+
PyThread_type_lock start_event;
2874+
PyThread_type_lock exit_event;
2875+
PyObject *callback;
2876+
} test_c_thread_t;
2877+
2878+
static void
2879+
temporary_c_thread(void *data)
2880+
{
2881+
test_c_thread_t *test_c_thread = data;
2882+
PyGILState_STATE state;
2883+
PyObject *res;
2884+
2885+
PyThread_release_lock(test_c_thread->start_event);
2886+
2887+
/* Allocate a Python thread state for this thread */
2888+
state = PyGILState_Ensure();
2889+
2890+
res = PyObject_CallFunction(test_c_thread->callback, "", NULL);
2891+
Py_CLEAR(test_c_thread->callback);
2892+
2893+
if (res == NULL) {
2894+
PyErr_Print();
2895+
}
2896+
else {
2897+
Py_DECREF(res);
2898+
}
2899+
2900+
/* Destroy the Python thread state for this thread */
2901+
PyGILState_Release(state);
2902+
2903+
PyThread_release_lock(test_c_thread->exit_event);
2904+
2905+
PyThread_exit_thread();
2906+
}
2907+
2908+
static PyObject *
2909+
call_in_temporary_c_thread(PyObject *self, PyObject *callback)
2910+
{
2911+
PyObject *res = NULL;
2912+
test_c_thread_t test_c_thread;
2913+
long thread;
2914+
2915+
PyEval_InitThreads();
2916+
2917+
test_c_thread.start_event = PyThread_allocate_lock();
2918+
test_c_thread.exit_event = PyThread_allocate_lock();
2919+
test_c_thread.callback = NULL;
2920+
if (!test_c_thread.start_event || !test_c_thread.exit_event) {
2921+
PyErr_SetString(PyExc_RuntimeError, "could not allocate lock");
2922+
goto exit;
2923+
}
2924+
2925+
Py_INCREF(callback);
2926+
test_c_thread.callback = callback;
2927+
2928+
PyThread_acquire_lock(test_c_thread.start_event, 1);
2929+
PyThread_acquire_lock(test_c_thread.exit_event, 1);
2930+
2931+
thread = PyThread_start_new_thread(temporary_c_thread, &test_c_thread);
2932+
if (thread == -1) {
2933+
PyErr_SetString(PyExc_RuntimeError, "unable to start the thread");
2934+
PyThread_release_lock(test_c_thread.start_event);
2935+
PyThread_release_lock(test_c_thread.exit_event);
2936+
goto exit;
2937+
}
2938+
2939+
PyThread_acquire_lock(test_c_thread.start_event, 1);
2940+
PyThread_release_lock(test_c_thread.start_event);
2941+
2942+
Py_BEGIN_ALLOW_THREADS
2943+
PyThread_acquire_lock(test_c_thread.exit_event, 1);
2944+
PyThread_release_lock(test_c_thread.exit_event);
2945+
Py_END_ALLOW_THREADS
2946+
2947+
Py_INCREF(Py_None);
2948+
res = Py_None;
2949+
2950+
exit:
2951+
Py_CLEAR(test_c_thread.callback);
2952+
if (test_c_thread.start_event)
2953+
PyThread_free_lock(test_c_thread.start_event);
2954+
if (test_c_thread.exit_event)
2955+
PyThread_free_lock(test_c_thread.exit_event);
2956+
return res;
2957+
}
2958+
28722959
static PyMethodDef TestMethods[] = {
28732960
{"raise_exception", raise_exception, METH_VARARGS},
28742961
{"raise_memoryerror", (PyCFunction)raise_memoryerror, METH_NOARGS},
@@ -2997,6 +3084,8 @@ static PyMethodDef TestMethods[] = {
29973084
{"docstring_with_signature_and_extra_newlines",
29983085
(PyCFunction)test_with_docstring, METH_NOARGS,
29993086
docstring_with_signature_and_extra_newlines},
3087+
{"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_O,
3088+
PyDoc_STR("set_error_class(error_class) -> None")},
30003089
{NULL, NULL} /* sentinel */
30013090
};
30023091

‎Objects/frameobject.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,6 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code, PyObject *globals,
726726
Py_INCREF(locals);
727727
f->f_locals = locals;
728728
}
729-
f->f_tstate = tstate;
730729

731730
f->f_lasti = -1;
732731
f->f_lineno = code->co_firstlineno;

0 commit comments

Comments
 (0)