From d0b41d1fc284bf94b9316166354cf2fea606676e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Jul 2022 11:40:02 -0700 Subject: [PATCH 1/4] Add failing regression test --- Lib/test/test_call.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index 6936f093e3db19..07355e8fa0616c 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -26,6 +26,18 @@ def fn(**kw): self.assertIsInstance(res, dict) self.assertEqual(list(res.items()), expected) + def test_frames_are_popped_after_failed_calls(self): + # GH-93252: stuff blows up if we don't pop the new frame after + # recovering from failed calls: + def f(): + pass + for _ in range(1000): + try: + f(None) + except TypeError: + pass + # BOOM! + @cpython_only class CFunctionCallsErrorMessages(unittest.TestCase): From e21751fb852c37d118aab1a3cdc2d6863c0e0b9b Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Jul 2022 11:41:09 -0700 Subject: [PATCH 2/4] Always pop the new frame after a failed call --- Python/ceval.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index 0176002432e8dd..602276143c3542 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -6410,7 +6410,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, } if (initialize_locals(tstate, func, localsarray, args, argcount, kwnames)) { assert(frame->owner != FRAME_OWNED_BY_GENERATOR); - _PyFrame_Clear(frame); + _PyEvalFrameClearAndPop(tstate, frame); return NULL; } return frame; @@ -6432,6 +6432,10 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, static void _PyEvalFrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame) { + PyObject **base = (PyObject **)frame; + // Make sure that this is, indeed, the top frame. We can't check this in + // _PyThreadState_PopFrame, since f_code is already cleared at that point: + assert(base + frame->f_code->co_framesize == tstate->datastack_top); tstate->recursion_remaining--; assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame); assert(frame->owner == FRAME_OWNED_BY_THREAD); From eb6f8c4eda250c32d60c47af829a0ef63491d833 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Jul 2022 11:47:31 -0700 Subject: [PATCH 3/4] blurb add --- .../2022-07-08-11-44-45.gh-issue-93252.i2358c.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-07-08-11-44-45.gh-issue-93252.i2358c.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-07-08-11-44-45.gh-issue-93252.i2358c.rst b/Misc/NEWS.d/next/Core and Builtins/2022-07-08-11-44-45.gh-issue-93252.i2358c.rst new file mode 100644 index 00000000000000..1cc2d8560e53e5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-07-08-11-44-45.gh-issue-93252.i2358c.rst @@ -0,0 +1,2 @@ +Fix an issue that caused internal frames to outlive failed Python function +calls, possibly resulting in memory leaks or hard interpreter crashes. From 5b46418fed959b72eda94ad7ada6485ba553b899 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 8 Jul 2022 12:27:02 -0700 Subject: [PATCH 4/4] Fix compiler warning --- Python/ceval.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 602276143c3542..2a0ce63d435d7c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -6432,10 +6432,10 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, static void _PyEvalFrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame) { - PyObject **base = (PyObject **)frame; // Make sure that this is, indeed, the top frame. We can't check this in // _PyThreadState_PopFrame, since f_code is already cleared at that point: - assert(base + frame->f_code->co_framesize == tstate->datastack_top); + assert((PyObject **)frame + frame->f_code->co_framesize == + tstate->datastack_top); tstate->recursion_remaining--; assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame); assert(frame->owner == FRAME_OWNED_BY_THREAD);