gh-135736: Fix interaction between TaskGroup and aclose()

[msullivan]

Currently if you have a generator like:

async def test():
    async with asyncio.TaskGroup() as tg:
        async for x in whatever():
            yield x

If aclose() is called on the generator and GeneratorExit is raised,
the TaskGroup's __aexit__ will raise a BaseExceptionGroup, and that
will be raised from the aclose() call instead of it being swallowed.

Fix this by raising GeneratorExit from aexit when:

1. The body of task group raised it
2. No subtasks raised exceptions

This makes GeneratorExit work without swallowing other exceptions
(like it would if we just added it to _is_base_error).

• Issue: GeneratorExit and asyncio.TaskGroup - missing from _is_base_error? #135736

[msullivan]

A slightly different approach that @1st1 and I discussed would be to add a class:

class TaskGroupGeneratorExit(GeneratorExit, BaseExceptionGroup):
    pass

and raise that, wrapping the original GeneratorExit, instead of just raising the original GeneratorExit.

This has the advantage that the behavior changes less: a BaseExceptionGroup still gets raised, it's just also now a GeneratorExit. I have an implementation of this and it works fine, but the idea of having to documen this exception for end users seemed unfortunate to me, so I didn't do it for this PR.

Any thoughts on this?

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #33803537 | 📁 Comparing 17bbd04 against main (998fc4a)

  🔍 Preview build  

19 files changed · ± 19 modified

± Modified

• license.html
• c-api/sys.html
• deprecations/index.html
• deprecations/pending-removal-in-3.20.html
• library/asyncio-eventloop.html
• library/asyncio-task.html
• library/ctypes.html
• library/curses.html
• library/difflib.html
• library/sys.html
• and 9 more...

[kumaraditya303]

This PR seems to do the same as #154538

[msullivan]

This PR seems to do the same as #154538

Huh, very funny that the bug sits around for a year and attracts two competing PRs in the same week.

Anyway, I think the approaches and behaviors of the two PRs are quite different, though.

I think this approach is preferable, because it doesn't drop exceptions on a GeneratorExit, which is not a particularly exceptional case. (Unlike SystemExit and KeyboardInterrupt)

[1st1]

because it doesn't drop exceptions on a GeneratorExit,

Yeah, that's quite important.

@kumaraditya303 Yeah, this PR has a better approach. Basically,

• A generator has a task group, all tasks completed, a GeneratorExit is thrown in -- we propagate just GeneratorExit, aclose() swallows it, no information lost.
• A generator has a task group, some tasks errored out, a GeneratorExit is thrown in -- we propagate an ExceptionGroup with GeneratorExit in it. aclose() would re-raise, no information is lost again. GeneratorExit ideally shouldn't be part of the propagated ExceptionGroup, but the fact that it's there isn't a big deal (removing it would require much more involved fix).

IMO this PR is fine.

We should salvage the logging part of https://github.com/python/cpython/pull/154538/changes in a separate PR though.

[1st1]

@msullivan I've approved the PR but I'll let it sit for some time before hitting the button in case Kumar or others have more comments.

[kumaraditya303]

I think we are backporting this, in that case it doesn't make sense to keep a version changed entry for 3.16

[msullivan]

Should I mark it for 3.15, then?
Or are we backporting further than that

[msullivan]

I've updated it to say 3.15. Let me know if you want something else

[kumaraditya303]

I think you should remove it entirely because it will be backported till 3.13.

[msullivan]

Would we just totally drop the new documentation of this, then?
It makes me nervous to have it documented there without any versionchanged notice?

We could also list the other point releases it is in; there's precedent for that in the docs (and in asyncio docs specifically)