Skip to content

Commit

Permalink
fix(coro): fix dpp::task's move assignment
Browse files Browse the repository at this point in the history
  • Loading branch information
Mishura4 committed Jul 13, 2024
1 parent d576711 commit f857554
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
2 changes: 1 addition & 1 deletion include/dpp/coro/awaitable.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ class promise_base {
value.template emplace<2>(std::move(ptr));
[[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel);
if constexpr (Notify) {
if (previous_value & sf_awaited) {
if ((previous_value & sf_awaited) != 0) {
this->awaiter.resume();
}
}
Expand Down
29 changes: 19 additions & 10 deletions include/dpp/coro/task.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,20 @@ class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it,
*/
explicit task(handle_t handle_) : awaitable<R>(&handle_.promise()), handle(handle_) {}

/**
* @brief Clean up our handle, cancelling any running task
*/
void cleanup() {
if (handle && this->valid()) {
if (this->abandon() & state_flags::sf_done) {
handle.destroy();
} else {
cancel();
}
handle = nullptr;
}
}

public:
/**
* @brief Default constructor, creates a task not bound to a coroutine.
Expand Down Expand Up @@ -136,6 +150,7 @@ class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it,
*/
task &operator=(task &&other) noexcept {
awaitable<R>::operator=(std::move(other));
cleanup();
handle = std::exchange(other.handle, nullptr);
return *this;
}
Expand All @@ -146,13 +161,7 @@ class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it,
* Destroys the handle. If the task is still running, it will be cancelled.
*/
~task() {
if (handle && this->valid()) {
if (this->abandon() & state_flags::sf_done) {
handle.destroy();
} else {
cancel();
}
}
cleanup();
}

/**
Expand Down Expand Up @@ -406,14 +415,14 @@ std_coroutine::coroutine_handle<> final_awaiter<R>::await_suspend(handle_t<R> ha
promise_t<R> &promise = handle.promise();
uint8_t previous_state = promise.state.fetch_or(state_flags::sf_done);

if (previous_state & state_flags::sf_awaited) { // co_await-ed, resume parent
if (previous_state & state_flags::sf_broken) { // major bug, these should never be set together
if ((previous_state & state_flags::sf_awaited) != 0) { // co_await-ed, resume parent
if ((previous_state & state_flags::sf_broken) != 0) { // major bug, these should never be set together
// we don't have a cluster so just log it on cerr
std::cerr << "dpp: task promise ended in both an awaited and dangling state. this is a bug and a memory leak, please report it to us!" << std::endl;
}
return promise.release_awaiter();
}
if (previous_state & state_flags::sf_broken) { // task object is gone, free the handle
if ((previous_state & state_flags::sf_broken) != 0) { // task object is gone, free the handle
handle.destroy();
}
return std_coroutine::noop_coroutine();
Expand Down

0 comments on commit f857554

Please sign in to comment.