originate_on_cancel(false) fails on the second co_await due to missed call, and does not work for IAsyncActions. - #1616
Open
antmor wants to merge 1 commit into
Open
Conversation
cancellable_promise exposes both a setter (originate_on_cancel) and a getter (should_originate_on_cancel), but both consumption sites called the setter. Because its parameter defaults to true, each guard evaluation performed std::exchange(m_originate_on_cancel, true): it returned the previous value, so the first check behaved correctly, and then wrote the flag back to true. Every later cancellation on that same promise originated again. A test that cancels only once passes even with the bug present. Three awaiter resume paths also threw hresult_canceled unconditionally, so originate_on_cancel(false) had no effect on them at all: impl::check_status_canceled (reached from await_adapter::await_resume for any coroutine awaiting a WinRT async that completes Canceled), timespan_awaiter::await_resume (resume_after) and signal_awaiter::await_resume (resume_on_signal). cancellable_awaiter now captures the promise's preference in await_suspend, where the promise is known to be alive, and consults it on resume. Capturing at suspend time rather than reaching for the promise at resume time keeps await_adapter::await_resume const. check_status_canceled takes an originate parameter defaulting to true, so wait_get and any external callers keep originating exactly as before. Adds async_originate_count_on_cancel, covering zero, single and repeated cancellation checks plus each of the three awaiter paths. It counts winrt_throw_hresult_handler invocations as a proxy for RoOriginateLanguageException, since observing the latter requires an out-of-process debugger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ecf487c-b73b-4c7c-940b-98920af404a5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug: #1617
originate_on_canceland gettershould_originate_on_cancel. The await_resume task would calloriginate_on_cancel(the setter) to check whether to RoOriginate the call. This is a bug, because the setter has a side effect. Its parameter defaults to true, so each call does std::exchange(m_originate_on_cancel, true): it returns the previous value, so the first check behaved correctly, and then wrote the flag back to true. Every later cancellation on that same promise originated again. A test that cancels only once passes even with the bug present.These three awaiter resume paths also threw hresult_canceled unconditionally, so originate_on_cancel(false) had no effect on them at all: impl::check_status_canceled (reached from await_adapter::await_resume for any coroutine awaiting a WinRT async that completes Canceled), timespan_awaiter::await_resume (resume_after) and signal_awaiter::await_resume (resume_on_signal).
Fix:
originate_on_cancel, call the getter instead.Test:
Adds async_originate_count_on_cancel, covering zero, single and repeated cancellation checks plus each of the three awaiter paths. It counts winrt_throw_hresult_handler invocations as a proxy for RoOriginateLanguageException, since observing the latter requires an out-of-process debugger.