Skip to content

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
microsoft:masterfrom
antmor:fix-cancellation-origination
Open

originate_on_cancel(false) fails on the second co_await due to missed call, and does not work for IAsyncActions.#1616
antmor wants to merge 1 commit into
microsoft:masterfrom
antmor:fix-cancellation-origination

Conversation

@antmor

@antmor antmor commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Bug: #1617

  1. Remove throwing/originating errors in expected scenarios (Lookup/TryLookup and Cancel) #1512 introduced the setter originate_on_cancel and getter should_originate_on_cancel. The await_resume task would call originate_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.
IAsyncAction DoWork(HANDLE ready)
{
    auto cancel = co_await get_cancellation_token();
    cancel.originate_on_cancel(false);   // "don't debug spew when I'm cancelled"

    co_await resume_on_signal(ready);    // If cancelled before `ready`, doesn't originate.
                                         // The first Cancel() consumes the opt-out, re-arms the flag
    co_await CleanupAsync();             // If cancelled after, then the next co_await will 
                                         // call Cancel() again, and since should_originate is true now
                                         // it will Originate, thus causing the debug spew.
}
  1. There was a missing scenario in the previous PR, it did not account for winrt::resume_after, winrt::resume_on_signal, and
    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:

  1. On callers that erroneously called the setter originate_on_cancel, call the getter instead.
  2. 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.

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.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant