From 8418dcc5d3190a199d2236e99e30f2c146939e95 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 15 Apr 2024 08:34:44 +0100 Subject: [PATCH 1/9] GH-117881: fix athrow().throw()/asend().throw() concurrent access --- Lib/test/test_asyncgen.py | 70 ++++++++++++++++++++++++++++++++++----- Objects/genobject.c | 36 ++++++++++++++++++++ 2 files changed, 97 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py index 39605dca3886c8..707bc03ce1db8a 100644 --- a/Lib/test/test_asyncgen.py +++ b/Lib/test/test_asyncgen.py @@ -393,6 +393,64 @@ async def gen(): r'anext\(\): asynchronous generator is already running'): an.__next__() + def test_async_gen_asend_throw_concurrent(self): + import types + + @types.coroutine + def _async_yield(v): + return (yield v) + + class MyExc(Exception): + pass + + async def agenfn(): + while True: + try: + await _async_yield(None) + except MyExc: + pass + return + yield + + + agen = agenfn() + gen = agen.asend(None) + gen.send(None) + gen2 = agen.asend(None) + + with self.assertRaisesRegex(RuntimeError, + r'anext\(\): asynchronous generator is already running'): + gen2.throw(MyExc) + + def test_async_gen_athrow_throw_concurrent(self): + import types + + @types.coroutine + def _async_yield(v): + return (yield v) + + class MyExc(Exception): + pass + + async def agenfn(): + while True: + try: + await _async_yield(None) + except MyExc: + pass + return + yield + + + agen = agenfn() + gen = agen.asend(None) + gen.send(None) + gen2 = agen.athrow(MyExc) + + with self.assertRaisesRegex(RuntimeError, + r'athrow\(\): asynchronous generator is already running'): + gen2.throw(MyExc) + def test_async_gen_3_arg_deprecation_warning(self): async def gen(): yield 123 @@ -1572,11 +1630,8 @@ async def main(): self.assertIsInstance(message['exception'], ZeroDivisionError) self.assertIn('unhandled exception during asyncio.run() shutdown', message['message']) - with self.assertWarnsRegex(RuntimeWarning, - f"coroutine method 'aclose' of '{async_iterate.__qualname__}' " - f"was never awaited"): - del message, messages - gc_collect() + del message, messages + gc_collect() def test_async_gen_expression_01(self): async def arange(n): @@ -1630,10 +1685,7 @@ async def main(): asyncio.run(main()) self.assertEqual([], messages) - with self.assertWarnsRegex(RuntimeWarning, - f"coroutine method 'aclose' of '{async_iterate.__qualname__}' " - f"was never awaited"): - gc_collect() + gc_collect() def test_async_gen_await_same_anext_coro_twice(self): async def async_iterate(): diff --git a/Objects/genobject.c b/Objects/genobject.c index 8d1dbb72ba9ec2..8cba9379b2158c 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -1771,6 +1771,7 @@ async_gen_asend_send(PyAsyncGenASend *o, PyObject *arg) if (o->ags_state == AWAITABLE_STATE_INIT) { if (o->ags_gen->ag_running_async) { + o->ags_state = AWAITABLE_STATE_CLOSED; PyErr_SetString( PyExc_RuntimeError, "anext(): asynchronous generator is already running"); @@ -1814,10 +1815,23 @@ async_gen_asend_throw(PyAsyncGenASend *o, PyObject *const *args, Py_ssize_t narg return NULL; } + if (o->ags_state == AWAITABLE_STATE_INIT) { + if (o->ags_gen->ag_running_async) { + o->ags_state = AWAITABLE_STATE_CLOSED; + PyErr_SetString( + PyExc_RuntimeError, + "anext(): asynchronous generator is already running"); + return NULL; + } + + o->ags_state = AWAITABLE_STATE_ITER; + } + result = gen_throw((PyGenObject*)o->ags_gen, args, nargs); result = async_gen_unwrap_value(o->ags_gen, result); if (result == NULL) { + o->ags_gen->ag_running_async = 0; o->ags_state = AWAITABLE_STATE_CLOSED; } @@ -2206,6 +2220,25 @@ async_gen_athrow_throw(PyAsyncGenAThrow *o, PyObject *const *args, Py_ssize_t na return NULL; } + if (o->agt_state == AWAITABLE_STATE_INIT) { + if (o->agt_gen->ag_running_async) { + o->agt_state = AWAITABLE_STATE_CLOSED; + if (o->agt_args == NULL) { + PyErr_SetString( + PyExc_RuntimeError, + "aclose(): asynchronous generator is already running"); + } + else { + PyErr_SetString( + PyExc_RuntimeError, + "athrow(): asynchronous generator is already running"); + } + return NULL; + } + + o->agt_state = AWAITABLE_STATE_ITER; + } + retval = gen_throw((PyGenObject*)o->agt_gen, args, nargs); if (o->agt_args) { return async_gen_unwrap_value(o->agt_gen, retval); @@ -2218,6 +2251,9 @@ async_gen_athrow_throw(PyAsyncGenAThrow *o, PyObject *const *args, Py_ssize_t na PyErr_SetString(PyExc_RuntimeError, ASYNC_GEN_IGNORED_EXIT_MSG); return NULL; } + if (retval == NULL) { + o->agt_gen->ag_running_async = 0; + } if (PyErr_ExceptionMatches(PyExc_StopAsyncIteration) || PyErr_ExceptionMatches(PyExc_GeneratorExit)) { From daf0f35f489e52ba946057931b1597cdebb8c1c8 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 15 Apr 2024 07:37:14 +0000 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst b/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst new file mode 100644 index 00000000000000..4f1e5e3ed8c828 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst @@ -0,0 +1 @@ +prevent concurrect access to an async generator via athrow().throw() or asend().throw() From 490e0a9da2653c9a1903ef2152d122daf650f791 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 15 Apr 2024 08:37:42 +0100 Subject: [PATCH 3/9] Update Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst --- .../2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst b/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst index 4f1e5e3ed8c828..75b34269695693 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-04-15-07-37-09.gh-issue-117881.07H0wI.rst @@ -1 +1 @@ -prevent concurrect access to an async generator via athrow().throw() or asend().throw() +prevent concurrent access to an async generator via athrow().throw() or asend().throw() From cbd302dda11c9fee7ee1e4293935e7e8426a7267 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 15 Apr 2024 09:05:41 +0100 Subject: [PATCH 4/9] Update Objects/genobject.c --- Objects/genobject.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 8cba9379b2158c..2be3efa4582e75 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -2241,7 +2241,11 @@ async_gen_athrow_throw(PyAsyncGenAThrow *o, PyObject *const *args, Py_ssize_t na retval = gen_throw((PyGenObject*)o->agt_gen, args, nargs); if (o->agt_args) { - return async_gen_unwrap_value(o->agt_gen, retval); + retval = async_gen_unwrap_value(o->agt_gen, retval); + if (retval == NULL) { + o->agt_gen->ag_running_async = 0; + } + return retval; } else { /* aclose() mode */ if (retval && _PyAsyncGenWrappedValue_CheckExact(retval)) { From 38b8246415f9f7323368ac633a1c3d32f282f41b Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 15 Apr 2024 09:09:24 +0100 Subject: [PATCH 5/9] set ag_running_async = 1 when calling async_gen_athrow_throw or async_gen_send_throw --- Objects/genobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/genobject.c b/Objects/genobject.c index 2be3efa4582e75..23d212750d3ae0 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -1825,6 +1825,7 @@ async_gen_asend_throw(PyAsyncGenASend *o, PyObject *const *args, Py_ssize_t narg } o->ags_state = AWAITABLE_STATE_ITER; + o->ags_gen->ag_running_async = 1; } result = gen_throw((PyGenObject*)o->ags_gen, args, nargs); @@ -2237,6 +2238,7 @@ async_gen_athrow_throw(PyAsyncGenAThrow *o, PyObject *const *args, Py_ssize_t na } o->agt_state = AWAITABLE_STATE_ITER; + o->agt_gen->ag_running_async = 1; } retval = gen_throw((PyGenObject*)o->agt_gen, args, nargs); From 144ddbc803f59e20e70ca844522bafd837e41296 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 15 Apr 2024 09:37:39 +0100 Subject: [PATCH 6/9] add tests for running asend().throw() and athrow().throw concurrent to themselves --- Lib/test/test_asyncgen.py | 71 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py index 707bc03ce1db8a..1660391b65f773 100644 --- a/Lib/test/test_asyncgen.py +++ b/Lib/test/test_asyncgen.py @@ -393,7 +393,7 @@ async def gen(): r'anext\(\): asynchronous generator is already running'): an.__next__() - def test_async_gen_asend_throw_concurrent(self): + def test_async_gen_asend_throw_concurrent_with_send(self): import types @types.coroutine @@ -422,7 +422,7 @@ async def agenfn(): r'anext\(\): asynchronous generator is already running'): gen2.throw(MyExc) - def test_async_gen_athrow_throw_concurrent(self): + def test_async_gen_athrow_throw_concurrent_with_send(self): import types @types.coroutine @@ -451,6 +451,73 @@ async def agenfn(): r'athrow\(\): asynchronous generator is already running'): gen2.throw(MyExc) + def test_async_gen_asend_throw_concurrent_with_throw(self): + import types + + @types.coroutine + def _async_yield(v): + return (yield v) + + class MyExc(Exception): + pass + + async def agenfn(): + try: + yield + except MyExc: + pass + while True: + try: + await _async_yield(None) + except MyExc: + pass + + + agen = agenfn() + with self.assertRaises(StopIteration): + agen.asend(None).send(None) + + gen = agen.athrow(MyExc) + gen.throw(MyExc) + gen2 = agen.asend(MyExc) + + with self.assertRaisesRegex(RuntimeError, + r'anext\(\): asynchronous generator is already running'): + gen2.throw(MyExc) + + def test_async_gen_athrow_throw_concurrent_with_throw(self): + import types + + @types.coroutine + def _async_yield(v): + return (yield v) + + class MyExc(Exception): + pass + + async def agenfn(): + try: + yield + except MyExc: + pass + while True: + try: + await _async_yield(None) + except MyExc: + pass + + agen = agenfn() + with self.assertRaises(StopIteration): + agen.asend(None).send(None) + + gen = agen.athrow(MyExc) + gen.throw(MyExc) + gen2 = agen.athrow(None) + + with self.assertRaisesRegex(RuntimeError, + r'athrow\(\): asynchronous generator is already running'): + gen2.throw(MyExc) + def test_async_gen_3_arg_deprecation_warning(self): async def gen(): yield 123 From 25d8a2768e92375ac1bd81bb3da1c0c39d22aebb Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 26 Apr 2024 10:23:55 +0100 Subject: [PATCH 7/9] actually del g/gc_collect() in test_aclose_throw --- Lib/test/test_asyncgen.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py index a8ce8187a11859..d386d0bd75e60f 100644 --- a/Lib/test/test_asyncgen.py +++ b/Lib/test/test_asyncgen.py @@ -1937,9 +1937,9 @@ class MyException(Exception): g = gen() with self.assertRaises(MyException): g.aclose().throw(MyException) - del g - gc_collect() + del g + gc_collect() # does not warn unawaited if __name__ == "__main__": unittest.main() From 1220aee3d7a1a8e72903abc815fdf96f31c5ce34 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 26 Apr 2024 10:26:57 +0100 Subject: [PATCH 8/9] add tests that check already running aclose/athrow get closed, and don't warn unawaited --- Lib/test/test_asyncgen.py | 67 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py index d386d0bd75e60f..b659d0e661e00e 100644 --- a/Lib/test/test_asyncgen.py +++ b/Lib/test/test_asyncgen.py @@ -393,6 +393,10 @@ async def gen(): r'anext\(\): asynchronous generator is already running'): an.__next__() + with self.assertRaisesRegex(RuntimeError, + r"cannot reuse already awaited __anext__\(\)/asend\(\)"): + an.send(None) + def test_async_gen_asend_throw_concurrent_with_send(self): import types @@ -422,6 +426,10 @@ async def agenfn(): r'anext\(\): asynchronous generator is already running'): gen2.throw(MyExc) + with self.assertRaisesRegex(RuntimeError, + r"cannot reuse already awaited __anext__\(\)/asend\(\)"): + gen2.send(None) + def test_async_gen_athrow_throw_concurrent_with_send(self): import types @@ -451,6 +459,10 @@ async def agenfn(): r'athrow\(\): asynchronous generator is already running'): gen2.throw(MyExc) + with self.assertRaisesRegex(RuntimeError, + r"cannot reuse already awaited aclose\(\)/athrow\(\)"): + gen2.send(None) + def test_async_gen_asend_throw_concurrent_with_throw(self): import types @@ -485,6 +497,10 @@ async def agenfn(): r'anext\(\): asynchronous generator is already running'): gen2.throw(MyExc) + with self.assertRaisesRegex(RuntimeError, + r"cannot reuse already awaited __anext__\(\)/asend\(\)"): + gen2.send(None) + def test_async_gen_athrow_throw_concurrent_with_throw(self): import types @@ -518,6 +534,10 @@ async def agenfn(): r'athrow\(\): asynchronous generator is already running'): gen2.throw(MyExc) + with self.assertRaisesRegex(RuntimeError, + r"cannot reuse already awaited aclose\(\)/athrow\(\)"): + gen2.send(None) + def test_async_gen_3_arg_deprecation_warning(self): async def gen(): yield 123 @@ -1941,5 +1961,52 @@ class MyException(Exception): del g gc_collect() # does not warn unawaited + def test_asend_send_already_running(self): + @types.coroutine + def _async_yield(v): + return (yield v) + + async def agenfn(): + while True: + await _async_yield(1) + return + yield + + agen = agenfn() + gen = agen.asend(None) + gen.send(None) + gen2 = agen.asend(None) + + with self.assertRaisesRegex(RuntimeError, + r'anext\(\): asynchronous generator is already running'): + gen2.send(None) + + del gen2 + gc_collect() # does not warn unawaited + + + def test_athrow_send_already_running(self): + @types.coroutine + def _async_yield(v): + return (yield v) + + async def agenfn(): + while True: + await _async_yield(1) + return + yield + + agen = agenfn() + gen = agen.asend(None) + gen.send(None) + gen2 = agen.athrow(Exception) + + with self.assertRaisesRegex(RuntimeError, + r'athrow\(\): asynchronous generator is already running'): + gen2.send(None) + + del gen2 + gc_collect() # does not warn unawaited + if __name__ == "__main__": unittest.main() From d1cedbe9cf1860dbd3470c87b6705140e6ec0c9b Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 26 Apr 2024 10:28:48 +0100 Subject: [PATCH 9/9] indentation nits --- Lib/test/test_asyncgen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncgen.py b/Lib/test/test_asyncgen.py index b659d0e661e00e..1985ede656e7a0 100644 --- a/Lib/test/test_asyncgen.py +++ b/Lib/test/test_asyncgen.py @@ -460,7 +460,7 @@ async def agenfn(): gen2.throw(MyExc) with self.assertRaisesRegex(RuntimeError, - r"cannot reuse already awaited aclose\(\)/athrow\(\)"): + r"cannot reuse already awaited aclose\(\)/athrow\(\)"): gen2.send(None) def test_async_gen_asend_throw_concurrent_with_throw(self):