In C++/WinRT, you shouldn’t destroy an object while you’re co_awaiting it

Raymond Chen

March 7th, 20244 0

A customer ran into a problem where they were crashing inside a co_await. Here’s a minimal version:

struct MyThing : winrt::implements<MyThing, winrt::IInspectable>
{
    winrt::IAsyncAction m_pendingAction{ nullptr };

    winrt::IAsyncAction DoSomethingAsync() {
        auto lifetime = get_strong();
        m_pendingAction = LongOperationAsync();
        co_await m_pendingAction;
        PostProcessing();
    }

    void Cancel() {
        if (m_pendingAction) {
            m_pendingAction.Cancel();
            m_pendingAction = nullptr;
        }
    }
};

With this class, you can ask it to Do­Something­Async(), and it will set into motion some long asynchronous operation, and then do some post-processing of the result. If you are impatient, you can call MyThing::Cancel() to give up on that long operation. For simplicity, we’ll assume that all calls occur on the same thread.

What the customer found was that after calling MyThing::Cancel(), the co_await crashed on a null pointer.

Some time ago, I set down some basic ground rules for function parameters, and one of them was that function parameters must remain stable for the duration of a function call.

In this case, it’s not so much a function call as it is a function suspension, so the rule doesn’t apply exactly, but the spirit is the same. The MyThing::Cancel() method cancels the m_pendingAction, which is fine, but it also modifies m_pendingAction out from under the co_await that is using it! When you call m_pendingAction.Cancel(), that cancels the LongOperationAsync(), which completes the IAsyncAction. At this point, the co_await will resume and call m_pendingAction.GetResults() to rethrown any errors.

But m_pendingAction was nulled out by the MyThing::Cancel(), so it’s calling GetResults() on a null pointer, which leads to the null pointer crash.

The solution here is not to co_await the member variable directly, but rather to make a copy and co_await the copy. One way to do that is to copy to a local and await the local.

    winrt::IAsyncAction DoSomethingAsync() {
        auto lifetime = get_strong();
        m_pendingAction = LongOperationAsync();
        // Await a copy of m_pendingAction because Cancel() 
        // modifies m_pendingAction.                        
        auto pendingAction = m_pendingAction;               
        co_await pendingAction;                             
        PostProcessing();
    }

Another is to create a copy as an inline temporary by doing a conversion to itself.

    winrt::IAsyncAction DoSomethingAsync() {
        auto lifetime = get_strong();
        m_pendingAction = LongOperationAsync();
        // Await a copy of m_pendingAction because Cancel() 
        // modifies m_pendingAction.                        
        co_await winrt::IAsyncAction(m_pendingAction);      
        PostProcessing();
    }

We’ll look some more at ways to force a copy next time.

Raymond Chen

Read next

4 comments

Discussion is closed. Login to edit/delete existing comments.

  • Antonio Rodríguez 1

    If you want to throw a pair of old gloves into the rubbish, make sure you aren’t wearing them. Or else your hands may get trapped in the bin.

  • Neil Rashbrook 1

    So what you’re saying is that co_await is a compiler transform that has the effect of taking m_pendingAction by reference rather than by value. I’ve probably misunderstood somewhere along the line because I thought that co_await needed an awaitable object, which a pointer itself isn’t. Either that or m_pendingAction isn’t a pointer, but it behaves like one, which in that case is just plain confusing to me.

    • Raymond ChenMicrosoft employee 0

      co_await is a compiler transform that takes an “awaiter”. Whether m_pendingAction is taken by reference or value depends on the awaiter. The awaiter for winrt::IAsyncAction takes the m_pendingAction by reference since the awaiter knows that it is always materialized as a temporary.

      • Neil Rashbrook 0

        Yes this is all down to me not understanding what the definition of the type of m_pendingAction actually is, which I need so that I can understand why the awaiter wants to take it by reference. (My best guess is that it’s actually a smart pointer, but even then my expectation would have been that the awaiter would take a weak reference to the underlying COM object and crash when it got destroyed by mistake.)

Feedback usabilla icon