calling destroy on a coroutine handle causes segfault

Question

I've recently begun experimenting with C++ coroutines using gcc-10.

The below code executes exactly the way it was intended to up till main exits, that destructs the task instance causing the _coro.destroy(); statement to segfault.

Why does this segfault?

#include <iostream>
#include <coroutine>

class task
{
public:
  struct promise_type;
  using handle = std::coroutine_handle<promise_type>;
  struct promise_type
  {
    std::suspend_never initial_suspend() const noexcept { return {}; }
    std::suspend_never final_suspend() const noexcept { return {}; }
    auto get_return_object() noexcept { return task{handle::from_promise(*this)}; }
    void unhandled_exception() { std::terminate(); }
  };

  ~task() { if (_coro != nullptr) { std::cout << "Destroying task\n"; _coro.destroy(); } }

  task(task const &) = delete; // Not implemented for brevity...
  task &operator=(task &&other) = delete;
  task(task &&rhs) = delete;

private:
  handle _coro = nullptr;
  task(handle coro) : _coro{coro} { std::cout << "Created task\n"; }
};

std::coroutine_handle<> resume_handle;

struct pause : std::suspend_always
{
  void await_suspend(std::coroutine_handle<> h)
  {
    resume_handle = h;
  }
};

task go()
{
  co_await pause();

  std::cout << "Finished firing\n";
}

int main(int argc, char *argv[])
{
  auto g = go();

  resume_handle();

  return 0;
}

Since your promise's `final_suspend` is `suspend_never`, you're saying "Allow the coroutine to self-destruct when finished." And then you destroy manually in the `~task` destructor, which is a double-free bug. — Raymond Chen, Jul 19 '20 at 18:50

Qix - MONICA WAS MISTREATED · Accepted Answer · 2020-10-31T15:03:24.810

A few things:

It's advised to use suspend_always or to return a coroutine_handle for resumption from final_suspend and then manually .destroy() the handle in the destructor.

~my_task() {
    if (handle) handle.destroy();
}

This is because returning suspend_never can cause the handle to be destructed for you, as Raymond Chen pointed out - which means you'd otherwise need to keep track of where a handle might be alive or might be destroyed, manually.

In my case, the segfault was due to a double-free, which can happen if the handle has been copied anywhere else. Remember that coroutine_handle<> is a non-owning handle to a coroutine's state, so multiple copies can point to the same (potentially-freed) memory.

Assuming you're not doing any sort of manual destructor invocation, make sure to also enforce that only one copy of your handle is in use at any given time.

Remember that copy semantics are going to be used in lieu of move semantics in a lot of cases, so you need to delete the copy constructor and assignment operators.

You must also define the move constructor since (as per cppreference, so take with a grain of salt) the move constructor on the coroutine_handle<> is the implicitly declared one, which means the internal pointer is merely copied and not set to nullptr (more info provided in this answer).

Therefore, a default move constructor will result in two completely "valid" (thus bool(handle) == true) coroutine_handle<> objects, causing multiple coroutine destructors to attempt to .destroy() a single coroutine instance, causing a double-free and thus a potential segfault.

class my_task {
    coroutine_handle<> handle;
public:
    inline my_task(my_task &&o) : handle(o.handle) {
        o.handle = nullptr; // IMPORTANT!
    }

    ~my_task() {
        if (handle) handle.destroy();
    }

    my_task(const my_task &) = delete;
    my_task & operator =(const my_task &) = delete;
};

Note that Valgrind is the tool of choice here for debugging such errors. As of this writing, the Valgrind provided in the Apt repositories is a bit out of date and doesn't have support for io_uring syscalls so it will choke with an error. Clone the latest master and build/install to get the patches for io_uring.

calling destroy on a coroutine handle causes segfault

1 Answers1