microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 238 forks source link

Bug: co_await for cancelled `IAsyncAction` resolves immediately, even if the coroutine is still running #1280

Closed fredemmott closed 1 year ago

fredemmott commented 1 year ago

Version

2.0.230225.1

Summary

Not sure if bug or feature request :)

I have coroutines which use RAII to manage ownership of global exclusive resources (e.g. named shared Direct3D textures); occasionally I need to shut them down, then start them up again.

Ideally, I would write:

auto storedAction = foo();
// ...
storedAction.Cancel();
try {
  // wait for RAII destructors etc
  co_await storedAction;
} catch (const winrt::hresult_canceled& e) {
  // ignore, expected
}
// can now assume that storedAction's RAII resources are gone
// ... but actually no, the coroutine could still be running

As a workaround, I'm doing:

winrt::handle completionHandle { CreateEvent(...) };
auto storedAction = foo(completionHandle.get);
// ...
storedAction.Cancel();
co_await winrt::resume_on_signal(completionHandle.get();
// Now the coroutine has actually finished executing

... where I have storedAction calling SetEvent() via RAII

Reproducible example

#include <winrt/base.h>
#include <winrt/Windows.Foundation.h>
#include <iostream>

using namespace std;
using namespace winrt;
using namespace winrt::Windows::Foundation;

#pragma comment(lib,"ole32.lib")
#pragma comment(lib,"oleaut32.lib")
#pragma comment(lib,"Kernel32.lib")

struct MyRAII {
    ~MyRAII() {
        cout << "~MyRAII" << endl;
    }
};

winrt::Windows::Foundation::IAsyncAction inner() {
    MyRAII it;

    auto token = co_await winrt::get_cancellation_token();
    token.callback([] { cout << "Cancellation callback" << endl; });

    // Does not do a difference
    token.enable_propagation();

    co_await winrt::resume_after(std::chrono::milliseconds(100));
}

winrt::Windows::Foundation::IAsyncAction outer() {
    auto action = inner();
    action.Cancel();
    try {
        co_await action;
    } catch (const winrt::hresult_canceled&) {
        cout << "Got hresult_canceled" << endl;
    }
    cout << "All done" << endl;
}

int main() {
    outer().get();
    return 0;
}

Expected behavior

Cancellation callback
~MyRAII
Got hresult_canceled
All done

co_await cancelled_iasyncaction only completes when the coroutine has finished executing

Actual behavior

Cancellation callback
Got hresult_canceled
All done

It appears to check that the IAsyncAction is marked as cancelled, then immediately returns without checking if it has actually finished executing

Additional comments

No response

sylveon commented 1 year ago

Does this also happen on 2.0.221121.5?

kennykerr commented 1 year ago

If y'all can point me to what's needed to get winrt coroutines working from int main without the framework, happy to give an isolated example

IAsyncOperation<int> Async() {
    co_return 123;
}

int main() {
    int result = Async().get();
    printf("%ld\n", result);
}
fredemmott commented 1 year ago

Added isolated example, and yes, also happens on 2.0.221121.5

kennykerr commented 1 year ago

Briefly, you'll have to synchronize this yourself as the coroutine's completion doesn't assume the coroutine itself has been torn down. In fact, it's pretty much guaranteed to not be torn down. What with the coroutine type being reference counted, the stack frame holding a reference, and the caller holding a reference, there's just no guarantee that completion will be gated in this way. Coroutine cancellation is also quite lossy or wishful at best so that too is not something you should rely on.

kennykerr commented 1 year ago

Another way to think about it is that a WinRT coroutine is really just a COM object with all of the locals within the coroutine being members of the COM object. Naturally, the COM object is reference counted and thus has a lifetime of its own and you shouldn't count on it being destroyed as long as there is an outstanding reference, such as the coroutine stack frame (that may be waiting on some resource).

Hope that helps.