Skip to content

gh-109955 : Update state transition comments for asyncio.Task #109910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 27, 2023

Conversation

kristjanvalur
Copy link
Contributor

@kristjanvalur kristjanvalur commented Sep 26, 2023

The in-line comments for the state transition of asyncio.Task objects has been out of date
for quite some time.

  • _fut_waiter persist in a done state on the Task between becoming done and until __step() is executed.
  • There is an intermediate state 2a where __wakeup() is scheduled, followed by 2b when __step() is scheduled.
  • An unmentioned state exists for the Task, namely the running state.
  • _step() and _wakeup() have been __step() and __wakeup() for quite some time.

This PR attempts to rectify the situation.

@kristjanvalur kristjanvalur changed the title Update state transition comments for asyncio.Task gh-109955 : Update state transition comments for asyncio.Task Sep 27, 2023
@kristjanvalur kristjanvalur marked this pull request as ready for review September 27, 2023 09:43
Comment on lines 90 to 91
# The transition from 2a to 2b happens when __wakeup() is executed,
# scheduling __step() to be called, leaving _fut_waiter in place.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This confuses me. Once __wakeup() executes, it proceeds without waiting to call __step() -- there's no scheduling involved that I can see in the source code (unless the .c extension is different). The call to future.result() immediately produces a value or raises -- it doesn't block (there's no await there). I do think it is possible to enter step 2b without going through 2a when __step() is scheduled directly by __init__(). So maybe it's clearer to say that we go through either 2a or 2b?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have got my wires crossed here, because it is all confusing. Initially, I thought __wakeup() was called directly (futures just calling registered callbacks) and _wakeup scheduling __step,.. But then it turns out that futures schedule their callbacks... I'll have another look.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you were right. So, there is no 2a or2b anymore just 2.
The main point I'm trying to get across here is that there can be a _fut_waiter present in a done state, i.e. the
mere presence of a _fut_waiter does not indicate that the Task is still blocked. I hope I'm not making this sound more complicated than it is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the mere presence of a _fut_waiter does not indicate that the Task is still blocked

Yes, that's crucial information.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge it now. Thanks!

@gvanrossum gvanrossum added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Sep 27, 2023
@gvanrossum gvanrossum merged commit 45cf5b0 into python:main Sep 27, 2023
@miss-islington
Copy link
Contributor

Thanks @kristjanvalur for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 27, 2023
…ythonGH-109910)

(cherry picked from commit 45cf5b0)

Co-authored-by: Kristján Valur Jónsson <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Sep 27, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 27, 2023
@bedevere-app
Copy link

bedevere-app bot commented Sep 27, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Sep 27, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 27, 2023
…ythonGH-109910)

(cherry picked from commit 45cf5b0)

Co-authored-by: Kristján Valur Jónsson <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
@kristjanvalur kristjanvalur deleted the kristjan/task-comments branch September 27, 2023 22:25
csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
vstinner pushed a commit that referenced this pull request Sep 29, 2023
…H-109910) (#109993)

gh-109955 : Update state transition comments for asyncio.Task (GH-109910)
(cherry picked from commit 45cf5b0)

Co-authored-by: Kristján Valur Jónsson <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Yhg1s pushed a commit that referenced this pull request Oct 2, 2023
…H-109910) (#109992)

gh-109955 : Update state transition comments for asyncio.Task (GH-109910)
(cherry picked from commit 45cf5b0)

Co-authored-by: Kristján Valur Jónsson <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants