* [Bug c++/98704] coroutine_handle::done() == false after promise's unhandled_exception() has thrown
2021-01-16 8:30 [Bug c++/98704] New: coroutine_handle::done() == false after promise's unhandled_exception() has thrown m.krause@tu-harburg.de
@ 2021-01-17 13:29 ` m.krause@tu-harburg.de
2021-03-12 15:28 ` iains at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: m.krause@tu-harburg.de @ 2021-01-17 13:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98704
--- Comment #1 from Michael Krause <m.krause@tu-harburg.de> ---
Created attachment 49985
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49985&action=edit
A possible fix?
Would this be the proper way to fix the problem? That is, instead of generating
a simple "promise.unhandled_exception()" call, generate:
try {
promise.unhandled_exception();
} catch(...) {
resume_fn_ptr = 0; // ensure that done() returns true
throw;
}
Seems to fix my problem, but then I don't know the gcc code base; maybe the
change has undesired side effects. If somebody can confirm this is the way to
go, I would be willing to provide a cleaned-up, mergable version including test
cases etc...
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/98704] coroutine_handle::done() == false after promise's unhandled_exception() has thrown
2021-01-16 8:30 [Bug c++/98704] New: coroutine_handle::done() == false after promise's unhandled_exception() has thrown m.krause@tu-harburg.de
2021-01-17 13:29 ` [Bug c++/98704] " m.krause@tu-harburg.de
@ 2021-03-12 15:28 ` iains at gcc dot gnu.org
2021-03-15 15:56 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: iains at gcc dot gnu.org @ 2021-03-12 15:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98704
Iain Sandoe <iains at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
Assignee|unassigned at gcc dot gnu.org |iains at gcc dot gnu.org
Last reconfirmed| |2021-03-12
--- Comment #2 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 50376
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50376&action=edit
Patch under test
Thanks for the report, and suggested fix.
This is the subject of CWG 2541 (about the wording, which is potentially
misleading).
Your possible fix doesn't do quite the right thing (because of said misleading
wording, I suspect) - it's necessary to ensure that the destroy() entry does
the correct thing when called.
=======
When promise.unhandled_exception () is entered, the coroutine is
considered to be still running - returning from the method will
cause the final await expression to be evaluated.
If the method throws, that action is considered to make the
coroutine suspend (since, otherwise, it would be impossible to
reclaim its resources, since one cannot destroy a running coro).
The wording issue is to do with how to represent the place at
which the coroutine should be considered suspended.
For the implementation here, that place is immediately before the
promise life-time ends. A handler for the rethrown exception, can
thus call xxxx.destroy() which will run DTORs for the promise and
any parameter copies [as needed] then the coroutine frame will be
deallocated.
At present, we also set "done=true" in this case (for compatibility
with other current implementations). One might consider 'done()'
to be misleading in the case of an abnormal termination - that is
also part of the CGW 2451 discussion.
I modified the reproducer into a test case that also checks that
the resources are properly cleaned up on an exceptional termination.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/98704] coroutine_handle::done() == false after promise's unhandled_exception() has thrown
2021-01-16 8:30 [Bug c++/98704] New: coroutine_handle::done() == false after promise's unhandled_exception() has thrown m.krause@tu-harburg.de
2021-01-17 13:29 ` [Bug c++/98704] " m.krause@tu-harburg.de
2021-03-12 15:28 ` iains at gcc dot gnu.org
@ 2021-03-15 15:56 ` cvs-commit at gcc dot gnu.org
2021-03-22 22:04 ` cvs-commit at gcc dot gnu.org
2021-03-24 12:40 ` iains at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-03-15 15:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98704
--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:
https://gcc.gnu.org/g:020b286c769f4dc8a6b45491351f6bc2e69d7a7f
commit r11-7676-g020b286c769f4dc8a6b45491351f6bc2e69d7a7f
Author: Iain Sandoe <iain@sandoe.co.uk>
Date: Thu Mar 11 17:04:14 2021 +0000
coroutines : Handle rethrow from unhandled_exception [PR98704].
Although there is still some discussion in CWG 2451 on this, the
implementors are agreed on the intent.
When promise.unhandled_exception () is entered, the coroutine is
considered to be still running - returning from the method will
cause the final await expression to be evaluated.
If the method throws, that action is considered to make the
coroutine suspend (since, otherwise, it would be impossible to
reclaim its resources, since one cannot destroy a running coro).
The wording issue is to do with how to represent the place at
which the coroutine should be considered suspended.
For the implementation here, that place is immediately before the
promise life-time ends. A handler for the rethrown exception, can
thus call xxxx.destroy() which will run DTORs for the promise and
any parameter copies [as needed] then the coroutine frame will be
deallocated.
At present, we also set "done=true" in this case (for compatibility
with other current implementations). One might consider 'done()'
to be misleading in the case of an abnormal termination - that is
also part of the CWG 2451 discussion.
gcc/cp/ChangeLog:
PR c++/98704
* coroutines.cc (build_actor_fn): Make destroy index 1
correspond to the abnormal unhandled_exception() exit.
Substitute the proxy for the resume index.
(coro_rewrite_function_body): Arrange to reset the resume
index and make done = true for a rethrown exception from
unhandled_exception ().
(morph_fn_to_coro): Adjust calls to build_actor_fn and
coro_rewrite_function_body.
gcc/testsuite/ChangeLog:
PR c++/98704
* g++.dg/coroutines/torture/pr98704.C: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/98704] coroutine_handle::done() == false after promise's unhandled_exception() has thrown
2021-01-16 8:30 [Bug c++/98704] New: coroutine_handle::done() == false after promise's unhandled_exception() has thrown m.krause@tu-harburg.de
` (2 preceding siblings ...)
2021-03-15 15:56 ` cvs-commit at gcc dot gnu.org
@ 2021-03-22 22:04 ` cvs-commit at gcc dot gnu.org
2021-03-24 12:40 ` iains at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-03-22 22:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98704
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:
https://gcc.gnu.org/g:07444226c6a3c78788aa967aad6889b1f31eceb0
commit r10-9517-g07444226c6a3c78788aa967aad6889b1f31eceb0
Author: Iain Sandoe <iain@sandoe.co.uk>
Date: Thu Mar 11 17:04:14 2021 +0000
coroutines : Handle rethrow from unhandled_exception [PR98704].
Although there is still some discussion in CWG 2451 on this, the
implementors are agreed on the intent.
When promise.unhandled_exception () is entered, the coroutine is
considered to be still running - returning from the method will
cause the final await expression to be evaluated.
If the method throws, that action is considered to make the
coroutine suspend (since, otherwise, it would be impossible to
reclaim its resources, since one cannot destroy a running coro).
The wording issue is to do with how to represent the place at
which the coroutine should be considered suspended.
For the implementation here, that place is immediately before the
promise life-time ends. A handler for the rethrown exception, can
thus call xxxx.destroy() which will run DTORs for the promise and
any parameter copies [as needed] then the coroutine frame will be
deallocated.
At present, we also set "done=true" in this case (for compatibility
with other current implementations). One might consider 'done()'
to be misleading in the case of an abnormal termination - that is
also part of the CWG 2451 discussion.
gcc/cp/ChangeLog:
PR c++/98704
* coroutines.cc (build_actor_fn): Make destroy index 1
correspond to the abnormal unhandled_exception() exit.
Substitute the proxy for the resume index.
(coro_rewrite_function_body): Arrange to reset the resume
index and make done = true for a rethrown exception from
unhandled_exception ().
(morph_fn_to_coro): Adjust calls to build_actor_fn and
coro_rewrite_function_body.
gcc/testsuite/ChangeLog:
PR c++/98704
* g++.dg/coroutines/torture/pr98704.C: New test.
(cherry picked from commit 020b286c769f4dc8a6b45491351f6bc2e69d7a7f)
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/98704] coroutine_handle::done() == false after promise's unhandled_exception() has thrown
2021-01-16 8:30 [Bug c++/98704] New: coroutine_handle::done() == false after promise's unhandled_exception() has thrown m.krause@tu-harburg.de
` (3 preceding siblings ...)
2021-03-22 22:04 ` cvs-commit at gcc dot gnu.org
@ 2021-03-24 12:40 ` iains at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: iains at gcc dot gnu.org @ 2021-03-24 12:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98704
Iain Sandoe <iains at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
fixed (according to the current agreement about done = true being set for a
coroutine suspended just before the promise lifetime end) for 10.3 and master.
^ permalink raw reply [flat|nested] 6+ messages in thread