https://github.com/python/cpython/commit/8e20e42cc63321dacc500d7670bfc225ca0... commit: 8e20e42cc63321dacc500d7670bfc225ca04e78b branch: main author: Mikhail Efimov <efimov.mikhail@gmail.com> committer: sobolevn <mail@sobolevn.me> date: 2025-01-22T15:50:01+03:00 summary: gh-125723: Fix crash with f_locals when generator frame outlive their generator (#126956) Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru> Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com> files: A Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst M Include/internal/pycore_object.h M Lib/test/test_generators.py M Objects/genobject.c diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 19d657070ff221..0b1df7e68b8dfa 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -62,7 +62,7 @@ extern void _Py_ForgetReference(PyObject *); PyAPI_FUNC(int) _PyObject_IsFreed(PyObject *); /* We need to maintain an internal copy of Py{Var}Object_HEAD_INIT to avoid - designated initializer conflicts in C++20. If we use the deinition in + designated initializer conflicts in C++20. If we use the definition in object.h, we will be mixing designated and non-designated initializers in pycore objects which is forbiddent in C++20. However, if we then use designated initializers in object.h then Extensions without designated break. diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 2ea6dba12effc1..b6985054c33d10 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -652,6 +652,89 @@ def genfn(): self.assertIsNone(f_wr()) +# See https://github.com/python/cpython/issues/125723 +class GeneratorDeallocTest(unittest.TestCase): + def test_frame_outlives_generator(self): + def g1(): + a = 42 + yield sys._getframe() + + def g2(): + a = 42 + yield + + def g3(obj): + a = 42 + obj.frame = sys._getframe() + yield + + class ObjectWithFrame(): + def __init__(self): + self.frame = None + + def get_frame(index): + if index == 1: + return next(g1()) + elif index == 2: + gen = g2() + next(gen) + return gen.gi_frame + elif index == 3: + obj = ObjectWithFrame() + next(g3(obj)) + return obj.frame + else: + return None + + for index in (1, 2, 3): + with self.subTest(index=index): + frame = get_frame(index) + frame_locals = frame.f_locals + self.assertIn('a', frame_locals) + self.assertEqual(frame_locals['a'], 42) + + def test_frame_locals_outlive_generator(self): + frame_locals1 = None + + def g1(): + nonlocal frame_locals1 + frame_locals1 = sys._getframe().f_locals + a = 42 + yield + + def g2(): + a = 42 + yield sys._getframe().f_locals + + def get_frame_locals(index): + if index == 1: + nonlocal frame_locals1 + next(g1()) + return frame_locals1 + if index == 2: + return next(g2()) + else: + return None + + for index in (1, 2): + with self.subTest(index=index): + frame_locals = get_frame_locals(index) + self.assertIn('a', frame_locals) + self.assertEqual(frame_locals['a'], 42) + + def test_frame_locals_outlive_generator_with_exec(self): + def g(): + a = 42 + yield locals(), sys._getframe().f_locals + + locals_ = {'g': g} + for i in range(10): + exec("snapshot, live_locals = next(g())", locals=locals_) + for l in (locals_['snapshot'], locals_['live_locals']): + self.assertIn('a', l) + self.assertEqual(l['a'], 42) + + class GeneratorThrowTest(unittest.TestCase): def test_exception_context_with_yield(self): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst new file mode 100644 index 00000000000000..62ca6f62f521a8 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst @@ -0,0 +1,2 @@ +Fix crash with ``gi_frame.f_locals`` when generator frames outlive their +generator. Patch by Mikhail Efimov. diff --git a/Objects/genobject.c b/Objects/genobject.c index b32140766c4a38..bc3a65d8aa25ec 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -134,6 +134,19 @@ _PyGen_Finalize(PyObject *self) PyErr_SetRaisedException(exc); } +static void +gen_clear_frame(PyGenObject *gen) +{ + if (gen->gi_frame_state == FRAME_CLEARED) + return; + + gen->gi_frame_state = FRAME_CLEARED; + _PyInterpreterFrame *frame = &gen->gi_iframe; + frame->previous = NULL; + _PyFrame_ClearExceptCode(frame); + _PyErr_ClearExcState(&gen->gi_exc_state); +} + static void gen_dealloc(PyObject *self) { @@ -159,13 +172,7 @@ gen_dealloc(PyObject *self) if (PyCoro_CheckExact(gen)) { Py_CLEAR(((PyCoroObject *)gen)->cr_origin_or_finalizer); } - if (gen->gi_frame_state != FRAME_CLEARED) { - _PyInterpreterFrame *frame = &gen->gi_iframe; - gen->gi_frame_state = FRAME_CLEARED; - frame->previous = NULL; - _PyFrame_ClearExceptCode(frame); - _PyErr_ClearExcState(&gen->gi_exc_state); - } + gen_clear_frame(gen); assert(gen->gi_exc_state.exc_value == NULL); PyStackRef_CLEAR(gen->gi_iframe.f_executable); Py_CLEAR(gen->gi_name); @@ -400,7 +407,7 @@ gen_close(PyObject *self, PyObject *args) // RESUME after YIELD_VALUE and exception depth is 1 assert((oparg & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START); gen->gi_frame_state = FRAME_COMPLETED; - _PyFrame_ClearLocals(&gen->gi_iframe); + gen_clear_frame(gen); Py_RETURN_NONE; } }