C++ coroutines: Is it valid to call `handle.destroy` from the final suspend point?

user_3043948 0
Viewed 550

Is it valid to call handle.destroy() from within the final suspension of a C++ coroutine?

From my understanding, this should be fine because the coroutine is currently suspended and it won't be resumed again.

Still, AddressSanitizer reports a heap-use-after-free for the following code snippet:

#include <experimental/coroutine>
#include <iostream>

using namespace std;

struct final_awaitable {
   bool await_ready() noexcept { return false; }
   void await_resume() noexcept {}
   template<typename PROMISE> std::experimental::coroutine_handle<> await_suspend(std::experimental::coroutine_handle<PROMISE> coro) noexcept {
      coro.destroy(); // Is this valid?
      return std::experimental::noop_coroutine();
   }
};

struct task {
   struct promise_type;
   using coro_handle = std::experimental::coroutine_handle<promise_type>;

   struct promise_type {
      task get_return_object() { return {}; }
      auto initial_suspend() { return std::experimental::suspend_never(); }
      auto final_suspend() noexcept { return final_awaitable(); }
      void unhandled_exception() { std::terminate(); }
      void return_void() {}
   };
};

task foo() {
    cerr << "foo\n";
    co_return;
}

int main() {
   auto x = foo();
}

when compiled with clang 11.0.1 and the compilation flags -stdlib=libc++ --std=c++17 -fcoroutines-ts -fno-exceptions -fsanitize=address. (see https://godbolt.org/z/eq6eoc)

(simplified version of my actual code. You can find the complete code in https://godbolt.org/z/8Yadv1)

Is this an issue in my code or a wrong positive in AddressSanitizer?

address-sanitizer c++-coroutine c++20 clang c++
1 Answers

It's perfectly valid if you're 100% sure nobody will use the coroutine promise afterwards. calling coroutine_handle::destroy is equivalent to calling the coroutine promise destructor.

If this is the case, then why to do it like this to begin with? just return std::suspend_never from final_suspend

std::suspend_never final_suspend() const noexcept { return {}; }

It's equivalent to your code. we want to suspend the coroutine in final_suspend if we want to do something meaningful with the coroutine promise after the coroutine is finished, like returning the stored result of the coroutine. since your task object doesn't store or return anything, I don't see why to final-suspend it.

Do note that if you use third-party libraries, like my concurrencpp, you need to make sure it's ok to destroy promises that are not yours. A coroutine promise might be suspended, but still referenced by its coroutine_handle somewhere else. This goes back to point #1. In the case of my library, it is not safe, because it could be that a result object still references it.

In conclusion, it's ok to call coroutine_promise::destroy if:

  1. the coroutine is suspended (when you reach final_suspend it is)
  2. nobody will use that coroutine promise after destruction (make extra sure there is no future-like object that references that coroutine!)
  3. destroy had not been called before (double-delete)
user_36135000
Related