microsoft / microsoft/cppwinrt

Bug: should_originate_on_cancel does not cancel certain awaiters or duplicated actions.

Open
#1,617 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
1.9k
Forks
281
PR merge metrics
No merged PRs in 30d

Description

Version

3.0.260715.1

Summary
  1. #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.

}
In addition, there is 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).

Reproducible example
IAsyncAction PollStatusAsync()
{
    auto cancel = co_await get_cancellation_token();
    cancel.enable_propagation();
    cancel.originate_on_cancel(false);

    while (true)
    {
        co_await RefreshAsync();
        co_await resume_after(30s);   // cancelled here -> timespan_awaiter::await_resume
    }                                 //    throws hresult_canceled() -> originates
}
Expected behavior

No response

Actual behavior

No response

Additional comments

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Trace cancellation handling through impl::check_status_canceled, await_adapter::await_resume, timespan_awaiter::await_resume, and signal_awaiter::await_resume. Verify that repeated cancellation does not restore the originate-on-cancel setting and that resume_after and resume_on_signal honor originate_on_cancel(false); add or update coverage for the repeated and listed awaiter scenarios.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.