Skip to content
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

Remove the final "done" state of subtasks from the async ABI #425

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

lukewagner
Copy link
Member

This PR has 2 commits, the first one is pure refactoring, no observable change (moving code from Subtask into canon_lower, which makes the logic easier to follow, imo), the second one removes CallState.DONE.

Before this PR, excusive ownership of borrowed handles is returned when a subtask (i.e., async import call) reaches the "done" state. Abstractly this made sense, but working through source-language bindings, there is no good way of expressing when this event happens in the syntax, and this is an important event to express well (since it determines whether dropping/transferring the borrowed handle traps). Syntactically, the natural place to return borrows is when the subtask returns its value, which is the "returned" state (which precedes the "done" state), since that maps to, e.g., when a Promise/Future is visibly resolved. So that's what this PR does.

With that change, there is no need for the "done" state, it just adds noise, so this PR removes the "done" state entirely, allowing the subtask to be dropped by its caller immediately after receiving the return value. This does mean that subtasks can continue executing (after task.return) even after their supertask finishes, but this doesn't break Structured Concurrency (and the existence of an unambiguous call-tree, which is semantically used by the reentrance guard) if we view the supertask as "async tail-calling" any still-executing subtasks when it exits. A blurb is added to Async.md by this PR explaining this more.

Two other notes:

  • The rules for lifting borrow are tweaked (in CanonicalABI.md) to account for the fact that a supertask can now return before its subtasks have returned, so simply checking that a borrow handle is passed to a nested subtask is no longer a sufficient condition to ensure proper nesting. However, the new rule is actually more expressive than the old rule, so it's probably what it should have been all along.
  • Since the current implementation plan does not allow borrow inside stream<T> or future<T> anyways, this PR changes the validation rules in the proposal to match, which is a conservative change we relax in the future if someone actually finds a use case for this (I'm not aware of any, atm).

Copy link
Collaborator

@dicej dicej left a comment

Choose a reason for hiding this comment

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

LGTM; thanks!

Regarding borrows, I think it would help to add a section in Async.md that enumerates how borrowed parameters interact with structured concurrency (i.e the rules which guests must follow and which hosts must enforce), especially now that we've removed the "done" state. Currently, the topic of borrows is spread across CanonicalABI.md such that you kind of have to read the whole thing to get the whole picture; it would be nice to have a succinct summary to refer people to if they have questions.

design/mvp/CanonicalABI.md Outdated Show resolved Hide resolved
@lukewagner
Copy link
Member Author

@dicej Great point; I added a blurb to Async.md in this commit.

@lukewagner lukewagner merged commit 7139a64 into main Dec 20, 2024
2 checks passed
@lukewagner lukewagner deleted the rm-done branch December 20, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants