C++ Logo

std-proposals

Advanced search

A resolution to a potential defect in coroutines

From: David Ledger <DavidLedger_at_[hidden]>
Date: Sat, 17 Oct 2020 05:22:25 +0000
Hello All,

Firstly, its great reading these emails, I wish I could contribute more to the discussions but for now I’m still learning the processes and accumulating standards knowledge. Today, I have a proposal to fix what is IMO a particularly irritating defect in coroutines (well it is for me 😉 …).

If I have the following function:

     Task<int> example()
     {
           co_yield 1;
           co_return 2; //could be co_yield (not relevant)
     }

And then use the function like this:

     auto fn = example();
     auto res = co_await fn + co_await fn

For MSVC and GCC the value in res is 4 (it should be 3).
It appears that the result of the expression is undefined behaviour (https://eel.is/c++draft/intro.execution#10).
Or it is at least the standard is ambiguous enough for GCC and MSVC implementers to assume re-ordering was alright or just not handle this specifically it at all.

When await_suspend returns coroutine_handle:
What appears to happen in GCC and MSVC is that a coroutine_handle returned by await_suspend isn’t immediately executed, instead the .resume call is reordered against other calls to await_suspend and resume on the same coroutine handle. Because the resume isn’t immediately executed, re-ordering allows the saved value inside the promise type to be overridden. This is a little crippling, its non-trivial to save the result of a co_return/co_yield into the awaitable (but would solve the issue).

When await_suspend returns void/bool:
GCC and MSVC still reorder the execution, identically to the case above. See https://godbolt.org/z/MG7n5z.

I propose the following change be made to resolve the defect (unless I’m wrong about it being a defect):

From https://eel.is/c++draft/intro.execution#10
---
(5.1) — If the result is false, the coroutine is considered suspended. Then, the await-suspend expression is evaluated. If that expression has type std::experimental::coroutine_handle and evaluates to value s, the coroutine referred to by s is +++immediately+++ resumed as if by a call s.resume() and then control flow returns to the current coroutine caller or resumer (8.4.4). Implementations shall not impose any limits on how many coroutines can be resumed in this fashion. If that expression has type bool and evaluates to false, the coroutine is resumed. If that expression exits via an exception, the exception is caught, the coroutine is resumed, and the exception is immediately re-thrown (15.1). Otherwise, control flow returns to the current coroutine caller or resumer (8.4.4) without exiting any scopes (6.6).
---

The change here is just the word immediately, I am hardly a standards guy so I assume there is a better way to handle the issue (but I want to get the discussion started). The proposed change does not resolve the problem where await_suspend returns bool/void. I don’t know how to solve that issue at this time…

The following post shows the reordering in action on MSVC and GCC (LLVM seems to behave as expected):
https://stackoverflow.com/questions/64348125/c20-coroutines-unexpected-reordering-of-await-resume-return-value-and-yield

Bug reports to MSVC and GCC here:
https://developercommunity.visualstudio.com/content/problem/1224166/c20-coroutine-promise-type-constructor-arguments-n.html
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97452
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97433

Kind Regards,
    David Ledger

Received on 2020-10-17 00:38:09