From 113a9dc697a0654a5bc7017fdc6f2cdfd788028f Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Mon, 18 Nov 2024 12:23:55 +0300 Subject: [PATCH 1/3] gh-125723: Fix crash w/ f_locals when generator frame outlive their generator --- Include/internal/pycore_object.h | 2 +- Lib/test/test_generators.py | 83 +++++++++++++++++++ ...-11-18-12-17-45.gh-issue-125723.tW_hFG.rst | 2 + Objects/genobject.c | 23 +++-- 4 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index c7af720b1ce43d..8fb2107d24b17a 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -61,7 +61,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 bf2cb1160723b0..c0b9113c8bad56 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..19b49274913a41 --- /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 frame outlive their +generator. Patch by Mikhail Efimov. diff --git a/Objects/genobject.c b/Objects/genobject.c index 19c2c4e3331a89..c90140edc69793 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; } } From d43ebe558a0d69155d35179ad6a1c90c1c62608d Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Mon, 18 Nov 2024 13:11:04 +0300 Subject: [PATCH 2/3] Update Lib/test/test_generators.py comment Co-authored-by: Kirill Podoprigora --- Lib/test/test_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index c0b9113c8bad56..855484175dd2ed 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -652,7 +652,7 @@ def genfn(): self.assertIsNone(f_wr()) -#See https://github.com/python/cpython/issues/125723 +# See https://github.com/python/cpython/issues/125723 class GeneratorDeallocTest(unittest.TestCase): def test_frame_outlives_generator(self): def g1(): From 38423a2fed4e584572a80c3eeebc399c8b18e225 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Wed, 22 Jan 2025 09:10:12 +0300 Subject: [PATCH 3/3] Update Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst Co-authored-by: Alyssa Coghlan --- .../2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 19b49274913a41..62ca6f62f521a8 100644 --- 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 @@ -1,2 +1,2 @@ -Fix crash with ``gi_frame.f_locals`` when generator frame outlive their +Fix crash with ``gi_frame.f_locals`` when generator frames outlive their generator. Patch by Mikhail Efimov.