`resume_agile` can be stored in a variable, resulting in hilarity

[oldnewthing]

Version

#1356

Summary

#1356 introduces resume_agile, but there is a design flaw. It saves the awaitable as a reference, expecting the usage to be

co_await resume_agile(DoSomething());

However, the caller might do this:

auto op = resume_agile(DoSomething());
co_await op;

This might happen when the developer is breaking down a complex expression into smaller pieces to help narrow down the source of a problem.

Storing the awaitable as a reference leads to a UAF in this case. We need to store the awaitable as a strong reference in the case where we are not preserving COM context. To avoid introducing unnecessary virtual calls to AddRef and Release, the argument to resume_agile can be forwarded into the await_adapter.

There is another defect in resume_agile if somebody awaits twice. This is not legal, but we should allow the illegal_delegate_assignment exception to propagate normally rather than triggering UB. A second call to await_suspend (newly possible thanks to resume_agile) would not set suspending back to true, causing the disconnect_aware_handler to try to resume the coroutine when the awaiter is destructed. But the coroutine is already going to be resumed by the C++ infrastructure as part of the "an exception was thrown from await_suspend" recovery. This results in double-resumption and hijinx ensue.

The fix is to set suspending to true at the start of await_suspend. This extra step is present only in the resume_agile code path. Throughout, we made sure to have no code generation effect on traditional co_await IAsyncSomething.

Reproducible example

auto op = resume_agile(DoSomething());
co_await op; // should not crash
co_await op; // should not exhibit UB

Expected behavior

First co_await awaits DoSomething() and resumes in any apartment. Second co_await throws illegal_delegate_assignment which can be caught by standard means.

Actual behavior

First co_await crashes. Second one double-resumes the coroutine and results in UB.

Additional comments

No response