From 8a8f20ff8cea201c9dcaf6967c51b2b210533723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 27 Dec 2024 13:01:31 +0100 Subject: [PATCH 1/7] clear PyExc_StopAsyncIteration exceptions in `_PyGen_SetStopIterationValue` --- Objects/genobject.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Objects/genobject.c b/Objects/genobject.c index e87f199c2504ba..78d601dddb8697 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -642,6 +642,22 @@ _PyGen_SetStopIterationValue(PyObject *value) PyErr_SetObject(PyExc_StopIteration, value); return 0; } + + // Since _PyGen_SetStopIterationValue() is meant to create + // a StopItertation or substitute one for a StopAsyncItertation + // an exception of another type should not already be set. + PyObject *run_ex = PyErr_Occurred(); + if (run_ex) { + if (!PyErr_GivenExceptionMatches(run_ex, PyExc_StopAsyncIteration)) { + // Replace existing bad exception with a SystemError instead. + PyErr_Format(PyExc_SystemError, + "%s:%d: unexpected caller exception: %R", + __FILE__, __LINE__, run_ex); + return -1; + } + PyErr_Clear(); + } + /* Construct an exception instance manually with * PyObject_CallOneArg and pass it to PyErr_SetObject. * From 0f4180bce2fc03eb4dfd1250bda6b1cb6004dc5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 27 Dec 2024 13:11:31 +0100 Subject: [PATCH 2/7] add tests --- Lib/test/test_asyncgen.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py index 5bfd789185c675..e9b741556ed0d5 100644 --- a/Lib/test/test_asyncgen.py +++ b/Lib/test/test_asyncgen.py @@ -1152,6 +1152,24 @@ async def run(): self.loop.run_until_complete(run()) + def test_async_gen_asyncio_anext_tuple_no_exceptions(self): + # StopAsyncIteration exceptions should be cleared. + # See: https://github.com/python/cpython/issues/128078. + + async def foo(): + if False: + yield (1, 2) + + async def run(): + it = foo().__aiter__() + with self.assertRaises(StopAsyncIteration): + await it.__anext__() + a, b = await anext(it, ('a', 'b')) + self.assertEqual(a, 'a') + self.assertEqual(b, 'b') + + self.loop.run_until_complete(run()) + def test_async_gen_asyncio_anext_stopiteration(self): async def foo(): try: From 1e5d9522aa0dcbb5b4e35b2c34ee6006cae7e2d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 27 Dec 2024 13:07:29 +0100 Subject: [PATCH 3/7] blurb --- .../2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst new file mode 100644 index 00000000000000..937da84541eb25 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst @@ -0,0 +1,2 @@ +Fix a :exc:`SystemError` when using :func:`anext` with a default tuple value. +Patch by Bénédikt Tran. From fed26a9f1c72cbe4804ff05c1232bc586f7d326f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 3 Jan 2025 11:57:27 +0100 Subject: [PATCH 4/7] correctly handle unexpected exceptions --- Objects/genobject.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 78d601dddb8697..300e381e37b401 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -643,20 +643,22 @@ _PyGen_SetStopIterationValue(PyObject *value) return 0; } - // Since _PyGen_SetStopIterationValue() is meant to create - // a StopItertation or substitute one for a StopAsyncItertation + // Since _PyGen_SetStopIterationValue() is called to create a + // StopItertation or substitute one for a StopAsyncItertation, // an exception of another type should not already be set. - PyObject *run_ex = PyErr_Occurred(); - if (run_ex) { - if (!PyErr_GivenExceptionMatches(run_ex, PyExc_StopAsyncIteration)) { + PyObject *old_exc = PyErr_GetRaisedException(); + if (old_exc) { + if (!PyErr_GivenExceptionMatches(old_exc, PyExc_StopAsyncIteration)) { // Replace existing bad exception with a SystemError instead. - PyErr_Format(PyExc_SystemError, - "%s:%d: unexpected caller exception: %R", - __FILE__, __LINE__, run_ex); + PyErr_BadInternalCall(); + // Set the previous bad exception to the cause of the SystemError. + PyObject *new_exc = PyErr_GetRaisedException(); + PyException_SetCause(new_exc, old_exc /* stolen */); + PyErr_SetRaisedException(new_exc); return -1; } - PyErr_Clear(); } + assert(!PyErr_Occurred()); /* Construct an exception instance manually with * PyObject_CallOneArg and pass it to PyErr_SetObject. From 3a49c8deba5554568337f46bab0e7dea97c601f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 9 Jan 2025 12:14:37 +0100 Subject: [PATCH 5/7] Simplify _PyGen_SetStopIterationValue() --- Objects/genobject.c | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 300e381e37b401..67c3ddc42de9d4 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -633,19 +633,10 @@ gen_iternext(PyObject *self) int _PyGen_SetStopIterationValue(PyObject *value) { - PyObject *e; - - if (value == NULL || - (!PyTuple_Check(value) && !PyExceptionInstance_Check(value))) - { - /* Delay exception instantiation if we can */ - PyErr_SetObject(PyExc_StopIteration, value); - return 0; - } - - // Since _PyGen_SetStopIterationValue() is called to create a - // StopItertation or substitute one for a StopAsyncItertation, - // an exception of another type should not already be set. + // Since _PyGen_SetStopIterationValue() must only be called to + // create a new StopIteration or substitute an existing one for + // a StopAsyncIteration, an exception of another type must not + // already be set. PyObject *old_exc = PyErr_GetRaisedException(); if (old_exc) { if (!PyErr_GivenExceptionMatches(old_exc, PyExc_StopAsyncIteration)) { @@ -654,27 +645,26 @@ _PyGen_SetStopIterationValue(PyObject *value) // Set the previous bad exception to the cause of the SystemError. PyObject *new_exc = PyErr_GetRaisedException(); PyException_SetCause(new_exc, old_exc /* stolen */); - PyErr_SetRaisedException(new_exc); + PyErr_SetRaisedException(new_exc /* stolen */); return -1; } + Py_DECREF(old_exc); } assert(!PyErr_Occurred()); - /* Construct an exception instance manually with - * PyObject_CallOneArg and pass it to PyErr_SetObject. - * - * We do this to handle a situation when "value" is a tuple, in which - * case PyErr_SetObject would set the value of StopIteration to - * the first element of the tuple. - * - * (See PyErr_SetObject/_PyErr_CreateException code for details.) - */ - e = PyObject_CallOneArg(PyExc_StopIteration, value); - if (e == NULL) { + // Construct an exception instance manually with PyObject_CallOneArg() + // but use PyErr_SetRaisedException() instead of PyErr_SetObject(). + // + // Indeed, PyErr_SetObject(exc_type, value) has a fast path when "value" + // is a tuple, where the value of the StopIteration exception would be + // set to `value[0]` instead of `value`. + PyObject *exc = value == NULL + ? PyObject_CallNoArgs(PyExc_StopIteration) + : PyObject_CallOneArg(PyExc_StopIteration, value); + if (exc == NULL) { return -1; } - PyErr_SetObject(PyExc_StopIteration, e); - Py_DECREF(e); + PyErr_SetRaisedException(exc /* stolen */); return 0; } From 5dbc72c137f78f4bd6e7e90098c15113959ddf69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 13 Jan 2025 14:28:21 +0100 Subject: [PATCH 6/7] Delete Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst --- .../2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst deleted file mode 100644 index 937da84541eb25..00000000000000 --- a/Misc/NEWS.d/next/Core_and_Builtins/2024-12-27-13-07-21.gh-issue-128078.xV1jpJ.rst +++ /dev/null @@ -1,2 +0,0 @@ -Fix a :exc:`SystemError` when using :func:`anext` with a default tuple value. -Patch by Bénédikt Tran. From 5ff8521d54f361de219f8981546765955d93258c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 13 Jan 2025 15:48:56 +0100 Subject: [PATCH 7/7] Update Objects/genobject.c Co-authored-by: Kumar Aditya --- Objects/genobject.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index c10c63a12f3c5a..b32140766c4a38 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -635,11 +635,10 @@ _PyGen_SetStopIterationValue(PyObject *value) { assert(!PyErr_Occurred()); // Construct an exception instance manually with PyObject_CallOneArg() - // but use PyErr_SetRaisedException() instead of PyErr_SetObject(). - // - // Indeed, PyErr_SetObject(exc_type, value) has a fast path when "value" + // but use PyErr_SetRaisedException() instead of PyErr_SetObject() as + // PyErr_SetObject(exc_type, value) has a fast path when 'value' // is a tuple, where the value of the StopIteration exception would be - // set to `value[0]` instead of `value`. + // set to 'value[0]' instead of 'value'. PyObject *exc = value == NULL ? PyObject_CallNoArgs(PyExc_StopIteration) : PyObject_CallOneArg(PyExc_StopIteration, value);