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

Allow resume_agile to be stored in a variable #1358

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

oldnewthing
Copy link
Member

resume_agile exposes the ability to save the await_adapter in a variable. This was not possible without resume_agile because the await_adapter had previously been available only via operator co_await, which means that it is created only in response to an immediate attempt to co_await it, so we knew that it would be consumed before its argument (possibly a temporary) was destructed.

resume_agile returns the await_adapter, and we expect people to await it immediately, but it's possible that they decide to save it in a variable and await it later. In that case, we have to record the Async as a value instead of a reference. We forward the resume_agile argument into the Async so that it moves if given an rvalue reference, or copies if given an lvalue reference. This ensure that the common case where somebody does co_await resume_agile(DoSomething()), we do not incur any additional AddRefs or Releases.

Now that it's possible to co_await the await_adapter twice, we have to worry about await_suspend being called twice. It had previously assumed that suspending was true (since that's how it was constructed), but that is no longer valid in the resume_agile case if somebody tries to await the resume_agile twice. So we have to force it to true. (Now, the second await will fail with "illegal delegate assignment", but our failure to set suspending to true led to double-resumption, which is super-bad.)

Fixes #1357

`resume_agile` exposes the ability to save the `await_adapter`
in a variable. This was not possible without `resume_agile` because
the `await_adapter` had previously been available only via
`operator co_await`, which means that it is created only in
response to an immediate attempt to `co_await` it, so we knew
that it would be consumed before its argument (possibly a temporary)
was destructed.

`resume_agile` returns the `await_adapter`, and we expect people
to await it immediately, but it's possible that they decide to
save it in a variable and await it later. In that case, we have
to record the `Async` as a value instead of a reference. We forward
the `resume_agile` argument into the `Async` so that it moves
if given an rvalue reference, or copies if given an lvalue reference.
This ensure that the common case where somebody does
`co_await resume_agile(DoSomething())`, we do not incur any additional
AddRefs or Releases.

Now that it's possible to `co_await` the `await_adapter` twice,
we have to worry about `await_suspend` being called twice. It had
previously assumed that `suspending` was true (since that's how it
was constructed), but that is no longer valid in the `resume_agile`
case if somebody tries to await the `resume_agile` twice. So we have
to force it to `true`. (Now, the second await will fail with
"illegal delegate assignment", but our failure to set `suspending`
to `true` led to double-resumption, which is super-bad.)
@oldnewthing oldnewthing merged commit bf4459b into microsoft:master Sep 14, 2023
75 checks passed
@oldnewthing oldnewthing deleted the resume-async2 branch September 14, 2023 23:19
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.

resume_agile can be stored in a variable, resulting in hilarity
3 participants