Calling destroy() from final_suspend() results in a crash

Question

I call h.destroy() in final_suspend to destroy the coroutine automatically when it finishes execution and then I resume awaiting coroutine (that awaits the task to complete). I found a question about this technique and an answer explaining why it should work.

As far as I can see, this technique really works, but not with MSVC 2022 that calls task destructor twice, see the code below:

#include <coroutine>
#include <optional>

#include <iostream>
#include <thread>

#include <chrono>
#include <queue>
#include <vector>

// simple timers

// stored timer tasks
struct timer_task
{
    std::chrono::steady_clock::time_point target_time;
    std::coroutine_handle<> handle;
};

// comparator
struct timer_task_before_cmp
{
    bool operator()(const timer_task& left, const timer_task& right) const
    {
        return left.target_time > right.target_time;
    }
};

std::priority_queue<timer_task, std::vector<timer_task>, timer_task_before_cmp> timers;

inline void submit_timer_task(std::coroutine_handle<> handle, std::chrono::nanoseconds timeout)
{
    timers.push(timer_task{ std::chrono::steady_clock::now() + timeout, handle });
}

//template <bool owning>
struct UpdatePromise;

//template <bool owning>
struct UpdateTask
{
    // declare promise type
    using promise_type = UpdatePromise;

    UpdateTask(std::coroutine_handle<promise_type> handle) :
        handle(handle)
    {
        std::cout << "UpdateTask constructor." << std::endl;
    }

    UpdateTask(const UpdateTask&) = delete;

    UpdateTask(UpdateTask&& other) : handle(other.handle)
    {
        std::cout << "UpdateTask move constructor." << std::endl;
    }

    UpdateTask& operator = (const UpdateTask&) = delete;

    UpdateTask& operator = (const UpdateTask&& other)
    {
        handle = other.handle;

        std::cout << "UpdateTask move assignment." << std::endl;

        return *this;
    }

    ~UpdateTask()
    {
        std::cout << "UpdateTask destructor." << std::endl;
    }

    std::coroutine_handle<promise_type> handle;
};

struct UpdatePromise
{
    std::coroutine_handle<> awaiting_coroutine;

    UpdateTask get_return_object();

    std::suspend_never initial_suspend()
    {
        return {};
    }

    void unhandled_exception()
    {
        std::terminate();
    }

    auto final_suspend() noexcept
    {
        // if there is a coroutine that is awaiting on this coroutine resume it
        struct transfer_awaitable
        {
            std::coroutine_handle<> awaiting_coroutine;

            // always stop at final suspend
            bool await_ready() noexcept
            {
                return false;
            }

            std::coroutine_handle<> await_suspend(std::coroutine_handle<UpdatePromise> h) noexcept
            {
                // resume awaiting coroutine or if there is no coroutine to resume return special coroutine that do
                // nothing
                std::coroutine_handle<> val = awaiting_coroutine ? awaiting_coroutine : std::noop_coroutine();

                h.destroy();

                return val;
            }

            void await_resume() noexcept {}
        };

        return transfer_awaitable{ awaiting_coroutine };
    }

    void return_void() {}

    // use `co_await std::chrono::seconds{n}` to wait specified amount of time
    auto await_transform(std::chrono::milliseconds d)
    {
        struct timer_awaitable
        {
            std::chrono::milliseconds m_d;

            // always suspend
            bool await_ready()
            {
                return m_d <= std::chrono::milliseconds(0);
            }

            // h is a handler for current coroutine which is suspended
            void await_suspend(std::coroutine_handle<> h)
            {
                // submit suspended coroutine to be resumed after timeout
                submit_timer_task(h, m_d);
            }
            void await_resume() {}
        };

        return timer_awaitable{ d };
    }

    // also we can await other UpdateTask<T>
    auto await_transform(UpdateTask& update_task)
    {
        if (!update_task.handle)
        {
            throw std::runtime_error("coroutine without promise awaited");
        }

        if (update_task.handle.promise().awaiting_coroutine)
        {
            throw std::runtime_error("coroutine already awaited");
        }

        struct task_awaitable
        {
            std::coroutine_handle<UpdatePromise> handle;

            // check if this UpdateTask already has value computed
            bool await_ready()
            {
                return handle.done();
            }

            // h - is a handle to coroutine that calls co_await
            // store coroutine handle to be resumed after computing UpdateTask value
            void await_suspend(std::coroutine_handle<> h)
            {
                handle.promise().awaiting_coroutine = h;
            }

            // when ready return value to a consumer
            auto await_resume()
            {
            }
        };

        return task_awaitable{ update_task.handle };
    }
};

inline UpdateTask UpdatePromise::get_return_object()
{
    return { std::coroutine_handle<UpdatePromise>::from_promise(*this) };
}

// timer loop
void loop()
{
    while (!timers.empty())
    {
        auto& timer = timers.top();
        // if it is time to run a coroutine
        if (timer.target_time < std::chrono::steady_clock::now())
        {
            auto handle = timer.handle;
            timers.pop();
            handle.resume();
        }
        else
        {
            std::this_thread::sleep_until(timer.target_time);
        }
    }
}

// example

using namespace std::chrono_literals;

UpdateTask TestTimerAwait()
{
    using namespace std::chrono_literals;

    std::cout << "testTimerAwait started." << std::endl;

    co_await 1s;

    std::cout << "testTimerAwait finished." << std::endl;
}

UpdateTask TestNestedTimerAwait()
{
    using namespace std::chrono_literals;

    std::cout << "testNestedTimerAwait started." << std::endl;

    auto task = TestTimerAwait();

    co_await 2s;

    //co_await task;

    std::cout << "testNestedTimerAwait finished." << std::endl;
}

// main can't be a coroutine and usually need some sort of looper (io_service or timer loop in this example)
int main()
{
    auto task = TestNestedTimerAwait();

    // execute deferred coroutines
    loop();
}

the output with MSVC 2022 is:

UpdateTask constructor.
testNestedTimerAwait started.
UpdateTask constructor.
testTimerAwait started.
testTimerAwait finished.
testNestedTimerAwait finished.
UpdateTask destructor.
UpdateTask destructor.
UpdateTask destructor.

but the output with GCC 11.1.0 is:

UpdateTask constructor.
testNestedTimerAwait started.
UpdateTask constructor.
testTimerAwait started.
testTimerAwait finished.
testNestedTimerAwait finished.
UpdateTask destructor.
UpdateTask destructor.

as you can see there is one extra destructor call with MSVC 2022, so the behaviour of the code generated with MSVC 2022 is undefined and it can potentially format your hard drive.

MSVC 2022 version: Microsoft (R) C/C++ Optimizing Compiler Version 19.30.30709 for x86

EDIT9:

Figured out what happens. The destructor of UpdateTask is called twice with MSVC 2022, see updated code.

EDIT10:

From docs: The coroutine is suspended (its coroutine state is populated with local variables and current suspension point). awaiter.await_suspend(handle) is called, where handle is the coroutine handle representing the current coroutine. Inside that function, the suspended coroutine state is observable via that handle, and it's this function's responsibility to schedule it to resume on some executor, or to be destroyed (returning false counts as scheduling)

Answer 1

Looks like it was a compiler bug, that is probably fixed in Microsoft (R) C/C++ Optimizing Compiler Version 19.31.31106.2 for x86, at least now the output is:

UpdateTask constructor.
testNestedTimerAwait started.
UpdateTask constructor.
testTimerAwait started.
testTimerAwait finished.
testNestedTimerAwait finished.
UpdateTask destructor.
UpdateTask destructor.