async generator allows concurrent access via async_gen_athrow_throw and async_gen_asend_throw

[graingert]

Bug report

Bug description:

import types
import itertools

@types.coroutine
def _async_yield(v):
    return (yield v)

class MyExc(Exception):
    pass

async def agenfn():
    for i in itertools.count():
        try:
            await _async_yield(i)
        except MyExc:
            pass
    return
    yield


agen = agenfn()
gen = agen.asend(None)
print(f"{gen.send(None)}")
gen2 = agen.asend(None)
try:
    print(f"{gen2.throw(MyExc)}")
except RuntimeError:
    print("good")
else:
    print("bad")

gen3 = agen.athrow(MyExc)
try:
    print(f"{gen3.throw(MyExc)}")
except RuntimeError:
    print("good")
else:
    print("bad")

outputs:

0
1
bad
2
bad

should print:

0
good
good

CPython versions tested on:

3.8, 3.9, 3.10, 3.11, 3.12, 3.13, CPython main branch

Operating systems tested on:

Linux

see also #7468 and #74956

Linked PRs

• GH-117881: fix athrow().throw()/asend().throw() concurrent access #117882
• [3.12] GH-117881: fix athrow().throw()/asend().throw() concurrent access (GH-117882) #118458

[graingert]

the problem is async_gen_athrow_throw and async_gen_asend_throw are not setting/checkingo->agt_gen->ag_running_async

[graingert]

a demo showing that this is reproducible using asyncio, as well as manual generator manipulation:

import asyncio

class MyExc(Exception):
    pass

async def main():
    task1 = None
    task2 = None

    async def agenfn():
        nonlocal task1
        nonlocal task2
        try:
            yield
        except asyncio.CancelledError:
            pass

        task1 = asyncio.current_task()
        try:
            await asyncio.sleep(0)
        except asyncio.CancelledError:
            task2 = asyncio.current_task()
            raise

    agen = agenfn()
    await anext(agen)
    try:
        async with asyncio.TaskGroup() as tg:
            tg.create_task(agen.athrow(MyExc))
            tg.create_task(agen.athrow(MyExc))
            raise MyExc
    except* MyExc:
        pass

    if task1 is not task2:
        print("task changed between yield calls!")


asyncio.run(main())

outputs:

task changed between yield calls!

[encukou]

I'm getting lost in this, and I guess I'm not the only one.
I started mapping out the state machine like this:

async_gen_unwrap_value:

    ag_running_async   ag_closed   result
    ------------------------------------------
    yes                (any)       - unchanged; no exception
                                   - ag_running_async=0; exception set
                                   - ag_running_async=0; ag_closed=1; exception set
    no                 (any)       shouldn't happen? (Should we add an `assert` for this?)


async_gen_asend_send:

    ags_state   ag_running_async   ag_closed   result
    -------------------------------------------------
    CLOSED      (any)              (any)       error: cannot reuse already awaited __anext__()/asend()
    INIT        yes                (any)       error: anext(): asynchronous generator is already running
    INIT        no                 (any)       ags_state=ITER; continue with the ags_state=ITER case
    ITER        (any)              (any)       - ag_running_async=1; no exception
                                               - ag_running_async=0; ags_state=CLOSED; exception set
                                               - ag_running_async=0; ag_closed=1; ags_state=CLOSED; exception set

but I remembered have other tasks lined up before 3.13 beta 1, and I probably shouldn't go too deep in this rabbit hole right now.

Do you think a map like this would help find similar bugs? Does something similar already exist?

[encukou]

@graingert Does my question make sense?

[graingert]

@encukou yes, I think it would be useful - where should it go? In the docs or just in a comment in genobject.c?

[encukou]

A comment is fine. If it's too long, it could be a separate document referenced from genobject.c.

Or possibly the devguide, if pretty rendering would be worth writing it in ReST. Not the user-facing docs though.