gh-111085: Fix invalid state handling in TaskGroup and Timeout #111111

serhiy-storchaka · 2023-10-20T12:42:36Z

asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeError if they are improperly used.

When they are used without entering the context manager.
When they are used after finishing.
When the context manager is entered more than once (simultaneously or sequentially).
If there is no current task when entering the context manager.

They are now left in consistent state after raising an exception, so following operations can be correctly performed (if they are allowed).

Issue: asyncio.Timeout raises AttributeError when not used in loop #111085

asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeError if they are improperly used. * When they are used without entering the context manager. * When they are used after finishing. * When the context manager is entered more than once (simultaneously or sequentially). * If there is no current task when entering the context manager. They are now left in consistent state after raising an exception, so following operations can be correctly performed (if they are allowed). Co-authored-by: James Hilton-Balfe <[email protected]>

Misc/NEWS.d/next/Library/2023-10-20-15-29-10.gh-issue-110910.u2oPwX.rst

…2oPwX.rst

gvanrossum

Thanks for doing this! The original code was adapted from various sources...

gvanrossum · 2023-10-20T20:51:58Z

Lib/asyncio/timeouts.py

@@ -82,11 +81,14 @@ def __repr__(self) -> str:
        return f"<Timeout [{self._state.value}]{info_str}>"

    async def __aenter__(self) -> "Timeout":
+        if self._state is not _State.CREATED:
+            raise RuntimeError(f"Timeout has been already {self._state.value}")


Looking at the list of states, this produces crooked grammar in several cases (active, expiring). Maybe "Timeout cannot be entered in state X"? Or look at the error in reschedule() for inspiration.

Thank you, I did not know that "already active" and "already expiring" are not valid. In TaskGroup it is simply "has been already entered" but I decided to enhance it. Technically, "already entered" should be enough.

reschedule() also produces weird message ("Cannot change state of created Timeout"). How can you change state of not created Timeout?

Lib/test/test_asyncio/test_timeouts.py

gvanrossum · 2023-10-20T20:56:00Z

Lib/test/test_asyncio/utils.py

+        try:
+            for _ in coro.__await__():
+                pass
+        except:


Maybe except BaseException as err: nonlocal exc; exc = err ?

…timeouts-states' into asyncio-taskgroups-timeouts-states

gvanrossum · 2023-10-20T22:51:28Z

Lib/asyncio/timeouts.py

@@ -82,7 +82,7 @@ def __repr__(self) -> str:

    async def __aenter__(self) -> "Timeout":
        if self._state is not _State.CREATED:
-            raise RuntimeError(f"Timeout has been already {self._state.value}")
+            raise RuntimeError("Timeout has been already entered")


Sorry, English grammar is hard. It should be "has already been entered".

gvanrossum

Bingo!

serhiy-storchaka · 2023-10-21T19:16:32Z

Thank you for your review Guido.

miss-islington-app · 2023-10-21T19:18:38Z

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

…ythonGH-111111) asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeError if they are improperly used. * When they are used without entering the context manager. * When they are used after finishing. * When the context manager is entered more than once (simultaneously or sequentially). * If there is no current task when entering the context manager. They now remain in a consistent state after an exception is thrown, so subsequent operations can be performed correctly (if they are allowed). (cherry picked from commit 6c23635) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: James Hilton-Balfe <[email protected]>

bedevere-app · 2023-10-21T19:18:51Z

GH-111171 is a backport of this pull request to the 3.12 branch.

bedevere-app · 2023-10-21T19:18:55Z

GH-111172 is a backport of this pull request to the 3.11 branch.

…GH-111111) (GH-111172) asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeError if they are improperly used. * When they are used without entering the context manager. * When they are used after finishing. * When the context manager is entered more than once (simultaneously or sequentially). * If there is no current task when entering the context manager. They now remain in a consistent state after an exception is thrown, so subsequent operations can be performed correctly (if they are allowed). (cherry picked from commit 6c23635) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: James Hilton-Balfe <[email protected]>

…GH-111111) (GH-111171) asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeError if they are improperly used. * When they are used without entering the context manager. * When they are used after finishing. * When the context manager is entered more than once (simultaneously or sequentially). * If there is no current task when entering the context manager. They now remain in a consistent state after an exception is thrown, so subsequent operations can be performed correctly (if they are allowed). (cherry picked from commit 6c23635) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: James Hilton-Balfe <[email protected]>

…ython#111111) asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeError if they are improperly used. * When they are used without entering the context manager. * When they are used after finishing. * When the context manager is entered more than once (simultaneously or sequentially). * If there is no current task when entering the context manager. They now remain in a consistent state after an exception is thrown, so subsequent operations can be performed correctly (if they are allowed). Co-authored-by: James Hilton-Balfe <[email protected]>

serhiy-storchaka added topic-asyncio needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 20, 2023

serhiy-storchaka requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners October 20, 2023 12:42

bedevere-app bot added the awaiting core review label Oct 20, 2023

serhiy-storchaka mentioned this pull request Oct 20, 2023

asyncio.Timeout raises AttributeError when not used in loop #111085

Closed

Gobot1234 mentioned this pull request Oct 20, 2023

gh-111085 Fix logic order causing nicer error to never be raised in asyncio.Timeout #110910

Closed

AlexWaygood changed the title ~~gh-110910: Fix invalid state handling in TaskGroup and Timeout~~ gh-111085: Fix invalid state handling in TaskGroup and Timeout Oct 20, 2023

hauntsaninja reviewed Oct 20, 2023

View reviewed changes

Misc/NEWS.d/next/Library/2023-10-20-15-29-10.gh-issue-110910.u2oPwX.rst Outdated Show resolved Hide resolved

Update Misc/NEWS.d/next/Library/2023-10-20-15-29-10.gh-issue-110910.u…

3127e26

…2oPwX.rst

gvanrossum reviewed Oct 20, 2023

View reviewed changes

serhiy-storchaka added 2 commits October 21, 2023 00:56

Address review comments.

5dfbb3d

Merge remote-tracking branch 'refs/remotes/origin/asyncio-taskgroups-…

ce732fb

…timeouts-states' into asyncio-taskgroups-timeouts-states

gvanrossum reviewed Oct 20, 2023

View reviewed changes

serhiy-storchaka added 3 commits October 21, 2023 09:24

Fix grammar.

0b0bdb0

Improve error message for not entered Timeout

0285c70

Add tests for expired and expiring Timeout.

95673e6

gvanrossum approved these changes Oct 21, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Oct 21, 2023

serhiy-storchaka merged commit 6c23635 into python:main Oct 21, 2023

bedevere-app bot removed the awaiting merge label Oct 21, 2023

serhiy-storchaka deleted the asyncio-taskgroups-timeouts-states branch October 21, 2023 19:18

bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 21, 2023

bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

gh-111085: Fix invalid state handling in TaskGroup and Timeout #111111

gh-111085: Fix invalid state handling in TaskGroup and Timeout #111111

Uh oh!

serhiy-storchaka commented Oct 20, 2023 •

edited by bedevere-app bot

Loading

Uh oh!

Uh oh!

gvanrossum left a comment

Uh oh!

gvanrossum Oct 20, 2023

Uh oh!

serhiy-storchaka Oct 20, 2023

Uh oh!

Uh oh!

gvanrossum Oct 20, 2023

Uh oh!

gvanrossum Oct 20, 2023

Uh oh!

gvanrossum left a comment

Uh oh!

serhiy-storchaka commented Oct 21, 2023

Uh oh!

miss-islington-app bot commented Oct 21, 2023

Uh oh!

bedevere-app bot commented Oct 21, 2023

Uh oh!

bedevere-app bot commented Oct 21, 2023

Uh oh!

Uh oh!

Uh oh!

gh-111085: Fix invalid state handling in TaskGroup and Timeout #111111

gh-111085: Fix invalid state handling in TaskGroup and Timeout #111111

Uh oh!

Conversation

serhiy-storchaka commented Oct 20, 2023 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gvanrossum Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

gvanrossum Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Oct 21, 2023

Uh oh!

miss-islington-app bot commented Oct 21, 2023

Uh oh!

bedevere-app bot commented Oct 21, 2023

Uh oh!

bedevere-app bot commented Oct 21, 2023

Uh oh!

Uh oh!

serhiy-storchaka commented Oct 20, 2023 •

edited by bedevere-app bot

Loading