MSVC cl 19.51 /O2: a coroutine destroyed at a final-statement suspension point leaks its by-value parameters

[christianparpart]

Summary

Under MSVC cl 19.51 at /O2, destroying a coroutine at a suspension point that has nothing after it in the body does not destroy the coroutine's by-value parameters. /Od is correct.

For a core::async::DetachedTask, "destroyed at a suspension point" is exactly what happens when an executor frees an abandoned chain at teardown: ParkedWork / AbandonState::release calls root.destroy(). So a detached handler whose last co_await is its final statement leaks every owning parameter it took by value when it is freed that way: a std::shared_ptr, a buffer, a sentinel. Nothing crashes. The destructors simply never run.

Found while testing 0.4.1's seal(), on release/next-041. The armed-claim cases in Strand_test.cpp and KeyedStrands_test.cpp checked that a chain is freed when the caller gives up its claim. They failed on cl-release only.

Minimal repro

#include <core/async/DetachedTask.hpp>
#include <core/async/ParkedWork.hpp>
#include <coroutine>
#include <cstdio>
#include <utility>

struct Sentinel
{
    int* destroyed;
    explicit Sentinel(int* d) noexcept: destroyed(d) {}
    Sentinel(Sentinel&& o) noexcept: destroyed(std::exchange(o.destroyed, nullptr)) {}
    ~Sentinel() { if (destroyed) ++*destroyed; }
};

struct ParkSelf
{
    std::coroutine_handle<>* self;
    bool await_ready() const noexcept { return false; }
    void await_suspend(std::coroutine_handle<> h) const noexcept { *self = h; }
    void await_resume() const noexcept {}
};

core::async::DetachedTask park(std::coroutine_handle<>* self, Sentinel s)
{
    (void) s;
    co_await ParkSelf { self };   // the last statement
}

int main()
{
    int destroyed = 0;
    std::coroutine_handle<> root;
    park(&root, Sentinel { &destroyed });
    auto work = core::async::ParkedWork { .resume = root, .abandon = core::async::detail::claimOn(root) };
    work.abandon.reset();          // the last claim on an armed chain: destroys the frame
    std::printf("destroyed = %d\n", destroyed);   // expected 1
}

cl /std:c++latest /EHsc /MD /I <core-cpp>/src:

Flags	destroyed
/Od	1
/O2	0: the frame is freed, the parameter's destructor never runs

A plain handle.destroy() on the suspended frame behaves the same way. The claim is incidental; it is simply how core-cpp's executors free an abandoned chain.

The shapes

Body after the parameter	/O2 result
(void) s; co_await ParkSelf { self }; (last statement)	parameter not destroyed
co_await ParkSelf { self }; (void) s;	parameter not destroyed
auto const held = std::move(s); co_await ParkSelf { self };	internal compiler error C1001
a Sentinel constructed as a body local instead of taken as a parameter	internal compiler error C1001
(void) s; co_await ParkSelf { self }; *self = {}; (any statement after the await)	correct: destroyed

So the "move it into a local" workaround does not work either: it crashes the compiler.

Consumer guidance until it is fixed

Under cl with optimisation:

• Keep a statement after the last co_await of a coroutine that may be destroyed while suspended there. Any statement will do; a no-op write is enough (see parkDetached in src/core/async/Strand_test.cpp, which carries a comment pointing here).
• Or take no owning parameters by value in such a coroutine: pass a pointer or a reference to state someone else owns.

Neither is needed for a coroutine that always runs to its end: parameters are destroyed correctly when the body completes. It is the destroy-while-suspended path, the one an executor's teardown takes, that leaks.

Relation to #51

This is a separate compiler defect with the same shape of trigger as #51: a DetachedTask whose lifetime ends at a suspension point, and generated code that mishandles the frame there.

• DetachedTask: under clang-cl -O0 the ramp reads its return object from a frame that may already be freed #51: clang-cl at -O0 reads the return object from a freed frame.
• This issue: cl at /O2 skips parameter destructors.

Whatever fix #51 chooses for DetachedTask should be checked against this repro as well.

Scope

• Seen with cl 19.51 (Visual Studio 18), x64, /O2. Other versions are not yet checked.
• clang-cl (both configurations), clang and GCC are unaffected.
• core-cpp's own tests work around it as above (release/next-041, 3454d84).
• Not reported upstream to Microsoft.