On 6/9/25 4:04 PM, Iain Sandoe wrote:
Hi Jason,
A complete re-implementation using a reference count as you suggested
in response to discussions of remaining issues. I also discussed
some of the points we encountered with one of the original coroutines
authors; it is accepted that having two places to cleanup was probably
a design flaw - but we are where we are. I added more cases to the
existing tests for exception cleanups (95615) and moved the original
115908 into the torture tests since it is for code-gen.
(this needs the can_throw fix or some of the 95615 cases will fail)
Tested on x86_64-darwin, powerpc64le-linux. OK for trunk?
thanks
Iain
--- 8< ---
This implements the final piece of the revised CWG2563 wording;
"It exits the scope of promise only if the coroutine completed
without suspending."
Considering the coroutine to be made up of two components; a
'ramp' and a 'body' where the body represents the user's original
code and the ramp is responsible for setup of that and for
returning some object to the original caller.
Coroutine state, and responsibility for its release.
A coroutine has some state that persists across suspensions.
The state has two components:
* State that is specified by the standard and persists for the entire
life of the coroutine.
* Local state that is constructed/destructed as scopes in the original
function body are entered/exited. The destruction of local state is
always the responsibility of the body code.
The persistent state (and the overall storage for the state) must be
managed in two places:
* The ramp function (which allocates and builds this - and can, in some
cases, be responsible for destroying it)
* The re-written function body which can destroy it when that body
completes its final suspend - or when the handle.destroy () is called.
In all cases the ramp holds responsibility for constructing the standard-
mandated persistent state.
There are four ways in which the ramp might be re-entered after starting
the function body:
A The body could suspend (one might expect that to be the 'normal' case
for most coroutine).
B The body might complete either synchronously or via continuations.
C An exception might be thrown during the setup of the initial await
expression, before the initial awaiter resumes.
D An exception might be processed by promise.unhandled_exception () and
that, in turn, might re-throw it (or throw something else). In this
case, the coroutine is considered suspended at the final suspension
point.
Once the coroutine has passed initial suspend (i.e. the initial awaiter
await_resume() has been called) the body is considered to have a use of
the state.
Until the ramp return value has been constructed, the ramp is considered
to have a use of the state.
To manage these interacting conditions we allocate a reference counter
for the frame state. This is initialised to 2 by the ramp as part of its
startup (note that failures/exceptions in the startup code are handled
locally to the ramp).
When the body completes by passing the final suspend, the reference is
decremented (the body no longer has a use). If the reference count is
now zero, then the body continues to destruction of the state. Passing
final suspend also corresponds to the handle.destroy () call.
In cases A and B the return path to the ramp decrements the count (since
the ramp return value can now be constructed and the ramp's use of the
state is complete).
In case C, when the exception arrives at the ramp, the reference count will
be 2. We cannot expect to construct the ramp return object.
Case D is an outlier, since the responsibility for destruction of the state
now rests with the user's code (via a handle.destroy() call). In the case
that the body has never suspended before such an exception occurs, the only
reasonable way for the user code to obtain the necessary handle is if
unhandled_exception() throws the handle or some object that contains the
handle. That is outside of the designs here - if the user code might need
this corner-case, then such provision will have to be made. Before the
unhandled_exception() is called, we save the refcount and then set it to
1 (since the only hold over the frame in the case of a thrown exception is
the body, the ramp return object cannot be constructed). The saved state
is restored if unhandled_exception returns normally and we carry on to
final_suspend.
In the ramp, we implement destruction for the persistent frame state by
means of cleanups. These are run conditionally when the reference count
is either 2 (signalling an exception before the body has fully started) or
0 signalling that both the body and the ramp have completed (i.e. the body
ran to completion before the ramp was re-entered).
PR c++/115908
PR c++/118074
PR c++/95615
gcc/cp/ChangeLog:
* coroutines.cc (coro_frame_refcount_id): New.
(coro_init_identifiers): Initialise coro_frame_refcount_id.
(build_actor_fn): Set up initial_await_resume_called. Handle
decrementing of the frame reference count. Return directly to
the caller if that is non-zero.
(cp_coroutine_transform::wrap_original_function_body): Save
and restore frame reference count across unhandled_exception.
(cp_coroutine_transform::build_ramp_function): Implement the
frame, promise and argument copy destruction via regular
cleanups. Decrement the reference count on the path that
constructs the ramp return object.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr115908.C: Move to...
* g++.dg/coroutines/torture/pr115908.C: ...here.
* g++.dg/coroutines/torture/pr95615-02.C: Move to...
* g++.dg/coroutines/torture/pr95615-01-promise-ctor-throws.C: ...here.
* g++.dg/coroutines/torture/pr95615-03.C: Move to...
* g++.dg/coroutines/torture/pr95615-02-get-return-object-throws.C:
...here.
* g++.dg/coroutines/torture/pr95615-01.C: Move to...
* g++.dg/coroutines/torture/pr95615-03-initial-suspend-throws.C:
...here.
* g++.dg/coroutines/torture/pr95615-04.C: Move to...
* g++.dg/coroutines/torture/pr95615-04-initial-await-ready-throws.C:
...here.
* g++.dg/coroutines/torture/pr95615-05.C: Move to...
* g++.dg/coroutines/torture/pr95615-05-initial-await-suspend-throws.C:
...here.
* g++.dg/coroutines/torture/pr95615.inc: Add more cases and ensure that
the
code completes properly when no exceptions are thrown.
* g++.dg/coroutines/torture/pr95615-00-nothing-throws.C: New test.
* g++.dg/coroutines/torture/pr95615-06-initial-await-resume-throws.C:
New test.
* g++.dg/coroutines/torture/pr95615-07-body-throws.C: New test.
*
g++.dg/coroutines/torture/pr95615-08-initial-suspend-throws-uhe-throws.C: New
test.
* g++.dg/coroutines/torture/pr95615-09-body-throws-uhe-throws.C: New
test.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 251 ++++++++++++++----
.../coroutines/{ => torture}/pr115908.C | 9 +-
.../torture/pr95615-00-nothing-throws.C | 5 +
...-02.C => pr95615-01-promise-ctor-throws.C} | 0
... => pr95615-02-get-return-object-throws.C} | 0
....C => pr95615-03-initial-suspend-throws.C} | 0
...> pr95615-04-initial-await-ready-throws.C} | 0
...pr95615-05-initial-await-suspend-throws.C} | 0
.../pr95615-06-initial-await-resume-throws.C | 7 +
.../torture/pr95615-07-body-throws.C | 7 +
...615-08-initial-suspend-throws-uhe-throws.C | 8 +
.../pr95615-09-body-throws-uhe-throws.C | 10 +
.../g++.dg/coroutines/torture/pr95615.inc | 196 ++++++++++----
13 files changed, 371 insertions(+), 122 deletions(-)
rename gcc/testsuite/g++.dg/coroutines/{ => torture}/pr115908.C (88%)
create mode 100644
gcc/testsuite/g++.dg/coroutines/torture/pr95615-00-nothing-throws.C
rename gcc/testsuite/g++.dg/coroutines/torture/{pr95615-02.C =>
pr95615-01-promise-ctor-throws.C} (100%)
rename gcc/testsuite/g++.dg/coroutines/torture/{pr95615-03.C =>
pr95615-02-get-return-object-throws.C} (100%)
rename gcc/testsuite/g++.dg/coroutines/torture/{pr95615-01.C =>
pr95615-03-initial-suspend-throws.C} (100%)
rename gcc/testsuite/g++.dg/coroutines/torture/{pr95615-04.C =>
pr95615-04-initial-await-ready-throws.C} (100%)
rename gcc/testsuite/g++.dg/coroutines/torture/{pr95615-05.C =>
pr95615-05-initial-await-suspend-throws.C} (100%)
create mode 100644
gcc/testsuite/g++.dg/coroutines/torture/pr95615-06-initial-await-resume-throws.C
create mode 100644
gcc/testsuite/g++.dg/coroutines/torture/pr95615-07-body-throws.C
create mode 100644
gcc/testsuite/g++.dg/coroutines/torture/pr95615-08-initial-suspend-throws-uhe-throws.C
create mode 100644
gcc/testsuite/g++.dg/coroutines/torture/pr95615-09-body-throws-uhe-throws.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index d82fa46f208..32fd1c65bf7 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -191,7 +191,87 @@ static bool coro_promise_type_found_p (tree, location_t);
just syntactic sugar for a co_await).
We defer the analysis and transformation until template expansion is
- complete so that we have complete types at that time. */
+ complete so that we have complete types at that time.
+
+ ---------------------------------------------------------------------------
+
+ Coroutine state, and responsibility for its release.
+
+ As noted above, a coroutine has some state that persists across suspensions.
+
+ The state has two components:
+ * State that is specified by the standard and persists for the entire
+ life of the coroutine.
+ * Local state that is constructed/destructed as scopes in the original
+ function body are entered/exited. The destruction of local state is
+ always the responsibility of the body code.
+
+ The persistent state (and the overall storage for the state) must be
+ managed in two places:
+ * The ramp function (which allocates and builds this - and can, in some
+ cases, be responsible for destroying it)
+ * The re-written function body which can destroy it when that body
+ completes its final suspend - or when the handle.destroy () is called.
+
+ In all cases the ramp holds responsibility for constructing the standard-
+ mandated persistent state.
+
+ There are four ways in which the ramp might be re-entered after starting
+ the function body:
+ A The body could suspend (one might expect that to be the 'normal' case
+ for most coroutine).
+ B The body might complete either synchronously or via continuations.
+ C An exception might be thrown during the setup of the initial await
+ expression, before the initial awaiter resumes.
+ D An exception might be processed by promise.unhandled_exception () and
+ that, in turn, might re-throw it (or throw something else). In this
+ case, the coroutine is considered suspended at the final suspension
+ point.
+
+ Once the coroutine has passed initial suspend (i.e. the initial awaiter
+ await_resume() has been called) the body is considered to have a use of
+ the state.
+
+ Until the ramp return value has been constructed, the ramp is considered
+ to have a use of the state.
+
+ To manage these interacting conditions we allocate a reference counter
+ for the frame state. This is initialised to 2 by the ramp as part of its
+ startup (note that failures/exceptions in the startup code are handled
+ locally to the ramp).
+
+ When the body completes by passing the final suspend, the reference is
+ decremented (the body no longer has a use). If the reference count is
+ now zero, then the body continues to destruction of the state. Passing
+ final suspend also corresponds to the handle.destroy () call.
+
+ In cases A and B the return path to the ramp decrements the count (since
+ the ramp return value can now be constructed and the ramp's use of the
+ state is complete).
+
+ In case C, when the exception arrives at the ramp, the reference count will
+ be 2. We cannot expect to construct the ramp return object.
+
+ Case D is an outlier, since the responsibility for destruction of the state
+ now rests with the user's code (via a handle.destroy() call). In the case
+ that the body has never suspended before such an exception occurs, the only
+ reasonable way for the user code to obtain the necessary handle is if
+ unhandled_exception() throws the handle or some object that contains the
+ handle. That is outside of the designs here - if the user code might need
+ this corner-case, then such provision will have to be made. Before the
+ unhandled_exception() is called, we save the refcount and then set it to
+ 1 (since the only hold over the frame in the case of a thrown exception is
+ the body, the ramp return object cannot be constructed). The saved state
+ is restored if unhandled_exception returns normally and we carry on to
+ final_suspend.
+
+ In the ramp, we implement destruction for the persistent frame state by
+ means of cleanups. These are run conditionally when the reference count
+ is either 2 (signalling an exception before the body has fully started) or
+ 0 signalling that both the body and the ramp have completed (i.e. the body
+ ran to completion before the ramp was re-entered).
I had figured that the ramp would set the reference count to 1
initially, and decrement it in a cleanup, and the actor would increment
it after initial await_resume, and decrement after final_suspend.
So then for A the count is 1 after the ramp decrement, and no cleanup is
done. For B both decrements happen, the count is 0, and the ramp cleans
up. For C the actor never incremented, so after the ramp decrement the
count is 0 and it cleans up. For D the count is 1 after the ramp
decrement, so the ramp does not clean up the state.
It shouldn't be necessary to add special refcount tweaking for case D if
we implement the ramp decrement as a cleanup rather than put it only on
the normal code path as in this patch.
Jason
