From a638d54c86e53e121e5085cd49846db3569d3718 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Wed, 1 Nov 2023 13:53:28 -0400 Subject: [PATCH 01/17] refactor(coro), feat(coro): coro tidy up, add sync_wait --- include/dpp/coro.h | 1 + include/dpp/coro/async.h | 443 ++------------------- include/dpp/coro/awaitable.h | 736 +++++++++++++++++++++++++++++++++++ include/dpp/coro/coroutine.h | 328 +++------------- include/dpp/coro/task.h | 492 ++++------------------- src/unittest/coro.cpp | 112 +++++- src/unittest/test.h | 1 + 7 files changed, 1007 insertions(+), 1106 deletions(-) create mode 100644 include/dpp/coro/awaitable.h diff --git a/include/dpp/coro.h b/include/dpp/coro.h index e1670e1acf..03d318119e 100644 --- a/include/dpp/coro.h +++ b/include/dpp/coro.h @@ -21,6 +21,7 @@ #pragma once +#include "coro/awaitable.h" #include "coro/async.h" #include "coro/coroutine.h" #include "coro/job.h" diff --git a/include/dpp/coro/async.h b/include/dpp/coro/async.h index 1863498cac..c5f8c283d9 100644 --- a/include/dpp/coro/async.h +++ b/include/dpp/coro/async.h @@ -22,10 +22,12 @@ #include +#include + namespace dpp { -struct async_dummy { - int* dummy_shared_state = nullptr; +struct async_dummy : awaitable_dummy { + std::shared_ptr dummy_shared_state = nullptr; }; } @@ -34,7 +36,6 @@ struct async_dummy { #include "coro.h" -#include #include #include #include @@ -45,341 +46,30 @@ namespace dpp { namespace detail { -/** - * @brief Empty struct used for overload resolution. - */ -struct empty_tag_t{}; - namespace async { /** - * @brief Represents the step an std::async is at. - */ -enum class state_t { - /** - * @brief Request was sent but not co_await-ed. handle is nullptr, result_storage is not constructed. - */ - sent, - - /** - * @brief Request was co_await-ed. handle is valid, result_storage is not constructed. - */ - waiting, - - /** - * @brief Request was completed. handle is unknown, result_storage is valid. - */ - done, - - /** - * @brief Request was never co_await-ed. - */ - dangling -}; - -/** - * @brief State of the async and its callback. - * - * Defined outside of dpp::async because this seems to work better with Intellisense. + * @brief Shared state of the async and its callback, to be used across threads. */ template -struct async_callback_data { - /** - * @brief Number of references to this callback state. - */ - std::atomic ref_count{1}; +struct callback { + std::shared_ptr> promise{nullptr}; - /** - * @brief State of the awaitable and the API callback - */ - std::atomic state = state_t::sent; - - /** - * @brief The stored result of the API call, stored as an array of bytes to directly construct in place - */ - alignas(R) std::array result_storage; - - /** - * @brief Handle to the coroutine co_await-ing on this API call - * - * @see std::coroutine_handle - */ - std_coroutine::coroutine_handle<> coro_handle = nullptr; - - /** - * @brief Convenience function to construct the result in the storage and initialize its lifetime - * - * @warning This is only a convenience function, ONLY CALL THIS IN THE CALLBACK, before setting state to done. - */ - template - void construct_result(Ts&&... ts) { - // Standard-compliant type punning yay - std::construct_at(reinterpret_cast(result_storage.data()), std::forward(ts)...); + void operator()(const R& v) const { + promise->set_value(v); } - /** - * @brief Destructor. - * - * Also destroys the result if present. - */ - ~async_callback_data() { - if (state.load() == state_t::done) { - std::destroy_at(reinterpret_cast(result_storage.data())); - } + void operator()(R&& v) const { + promise->set_value(std::move(v)); } }; -/** - * @brief Base class of dpp::async. - * - * @warning This class should not be used directly by a user, use dpp::async instead. - * @note This class contains all the functions used internally by co_await. It is intentionally opaque and a private base of dpp::async so a user cannot call await_suspend and await_resume directly. - */ -template -class async_base { - /** - * @brief Ref-counted callback, contains the callback logic and manages the lifetime of the callback data over multiple threads. - */ - struct shared_callback { - /** - * @brief Self-managed ref-counted pointer to the state data - */ - async_callback_data *state = new async_callback_data; - - /** - * @brief Callback function. - * - * Constructs the callback data, and if the coroutine was awaiting, resume it - * @param cback The result of the API call. - * @tparam V Forwarding reference convertible to R - */ - template V> - void operator()(V &&cback) const { - state->construct_result(std::forward(cback)); - if (auto previous_state = state->state.exchange(state_t::done); previous_state == state_t::waiting) { - state->coro_handle.resume(); - } - } - - /** - * @brief Main constructor, allocates a new callback_state object. - */ - shared_callback() = default; - - /** - * @brief Empty constructor, holds no state. - */ - explicit shared_callback(detail::empty_tag_t) noexcept : state{nullptr} {} +template <> +struct callback { + std::shared_ptr> promise{nullptr}; - /** - * @brief Copy constructor. Takes shared ownership of the callback state, increasing the reference count. - */ - shared_callback(const shared_callback &other) noexcept { - this->operator=(other); - } - - /** - * @brief Move constructor. Transfers ownership from another object, leaving intact the reference count. The other object releases the callback state. - */ - shared_callback(shared_callback &&other) noexcept { - this->operator=(std::move(other)); - } - - /** - * @brief Destructor. Releases the held reference and destroys if no other references exist. - */ - ~shared_callback() { - if (!state) { // Moved-from object - return; - } - - auto count = state->ref_count.fetch_sub(1); - if (count == 0) { - delete state; - } - } - - /** - * @brief Copy assignment. Takes shared ownership of the callback state, increasing the reference count. - */ - shared_callback &operator=(const shared_callback &other) noexcept { - state = other.state; - ++state->ref_count; - return *this; - } - - /** - * @brief Move assignment. Transfers ownership from another object, leaving intact the reference count. The other object releases the callback state. - */ - shared_callback &operator=(shared_callback &&other) noexcept { - state = std::exchange(other.state, nullptr); - return *this; - } - - /** - * @brief Function called by the async when it is destroyed when it was never co_awaited, signals to the callback to abort. - */ - void set_dangling() noexcept { - if (!state) { // moved-from object - return; - } - state->state.store(state_t::dangling); - } - - bool done(std::memory_order order = std::memory_order_seq_cst) const noexcept { - return (state->state.load(order) == state_t::done); - } - - /** - * @brief Convenience function to get the shared callback state's result. - * - * @warning It is UB to call this on a callback whose state is anything else but state_t::done. - */ - R &get_result() noexcept { - assert(state && done()); - return (*reinterpret_cast(state->result_storage.data())); - } - - /** - * @brief Convenience function to get the shared callback state's result. - * - * @warning It is UB to call this on a callback whose state is anything else but state_t::done. - */ - const R &get_result() const noexcept { - assert(state && done()); - return (*reinterpret_cast(state->result_storage.data())); - } - }; - - /** - * @brief Shared state of the async and its callback, to be used across threads. - */ - shared_callback api_callback{nullptr}; - -public: - /** - * @brief Construct an async object wrapping an object method, the call is made immediately by forwarding to std::invoke and can be awaited later to retrieve the result. - * - * @param obj The object to call the method on - * @param fun The method of the object to call. Its last parameter must be a callback taking a parameter of type R - * @param args Parameters to pass to the method, excluding the callback - */ - template -#ifndef _DOXYGEN_ - requires std::invocable> -#endif - explicit async_base(Obj &&obj, Fun &&fun, Args&&... args) : api_callback{} { - std::invoke(std::forward(fun), std::forward(obj), std::forward(args)..., api_callback); - } - - /** - * @brief Construct an async object wrapping an invokeable object, the call is made immediately by forwarding to std::invoke and can be awaited later to retrieve the result. - * - * @param fun The object to call using std::invoke. Its last parameter must be a callable taking a parameter of type R - * @param args Parameters to pass to the object, excluding the callback - */ - template -#ifndef _DOXYGEN_ - requires std::invocable> -#endif - explicit async_base(Fun &&fun, Args&&... args) : api_callback{} { - std::invoke(std::forward(fun), std::forward(args)..., api_callback); - } - - /** - * @brief Construct an empty async. Using `co_await` on an empty async is undefined behavior. - */ - async_base() noexcept : api_callback{detail::empty_tag_t{}} {} - - /** - * @brief Destructor. If any callback is pending it will be aborted. - */ - ~async_base() { - api_callback.set_dangling(); - } - - /** - * @brief Copy constructor is disabled - */ - async_base(const async_base &) = delete; - - /** - * @brief Move constructor - * - * NOTE: Despite being marked noexcept, this function uses std::lock_guard which may throw. The implementation assumes this can never happen, hence noexcept. Report it if it does, as that would be a bug. - * - * @remark Using the moved-from async after this function is undefined behavior. - * @param other The async object to move the data from. - */ - async_base(async_base &&other) noexcept = default; - - /** - * @brief Copy assignment is disabled - */ - async_base &operator=(const async_base &) = delete; - - /** - * @brief Move assignment operator. - * - * NOTE: Despite being marked noexcept, this function uses std::lock_guard which may throw. The implementation assumes this can never happen, hence noexcept. Report it if it does, as that would be a bug. - * - * @remark Using the moved-from async after this function is undefined behavior. - * @param other The async object to move the data from - */ - async_base &operator=(async_base &&other) noexcept = default; - - /** - * @brief Check whether or not co_await-ing this would suspend the caller, i.e. if we have the result or not - * - * @return bool Whether we already have the result of the API call or not - */ - [[nodiscard]] bool await_ready() const noexcept { - return api_callback.done(); - } - - /** - * @brief Second function called by the standard library when the object is co-awaited, if await_ready returned false. - * - * Checks again for the presence of the result, if absent, signals to suspend and keep track of the calling coroutine for the callback to resume. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @param caller The handle to the coroutine co_await-ing and being suspended - */ - [[nodiscard]] bool await_suspend(detail::std_coroutine::coroutine_handle<> caller) noexcept { - auto sent = state_t::sent; - api_callback.state->coro_handle = caller; - return api_callback.state->state.compare_exchange_strong(sent, state_t::waiting); // true (suspend) if `sent` was replaced with `waiting` -- false (resume) if the value was not `sent` (`done` is the only other option) - } - - /** - * @brief Function called by the standard library when the async is resumed. Its return value is what the whole co_await expression evaluates to - * - * @remark Do not call this manually, use the co_await keyword instead. - * @return The result of the API call as an lvalue reference. - */ - R& await_resume() & noexcept { - return api_callback.get_result(); - } - - - /** - * @brief Function called by the standard library when the async is resumed. Its return value is what the whole co_await expression evaluates to - * - * @remark Do not call this manually, use the co_await keyword instead. - * @return The result of the API call as a const lvalue reference. - */ - const R& await_resume() const& noexcept { - return api_callback.get_result(); - } - - /** - * @brief Function called by the standard library when the async is resumed. Its return value is what the whole co_await expression evaluates to - * - * @remark Do not call this manually, use the co_await keyword instead. - * @return The result of the API call as an rvalue reference. - */ - R&& await_resume() && noexcept { - return std::move(api_callback.get_result()); + void operator()() const { + promise->set_value(); } }; @@ -400,19 +90,16 @@ struct confirmation_callback_t; * @tparam R The return type of the API call. Defaults to confirmation_callback_t */ template -class async : private detail::async::async_base { - /** - * @brief Internal use only base class. It serves to prevent await_suspend and await_resume from being used directly. - * - * @warning For internal use only, do not use. - * @see operator co_await() - */ - friend class detail::async::async_base; +class async : public awaitable { + + detail::async::callback api_callback{}; + + explicit async(std::shared_ptr> &&promise) : awaitable{promise.get()}, api_callback{std::move(promise)} {} public: - using detail::async::async_base::async_base; // use async_base's constructors. unfortunately on clang this doesn't include the templated ones so we have to delegate below - using detail::async::async_base::operator=; // use async_base's assignment operator - using detail::async::async_base::await_ready; // expose await_ready as public + using awaitable::awaitable; // use awaitable's constructors + using awaitable::operator=; // use async_base's assignment operator + using awaitable::await_ready; // expose await_ready as public /** * @brief The return type of the API call. Defaults to confirmation_callback_t @@ -430,7 +117,9 @@ class async : private detail::async::async_base { #ifndef _DOXYGEN_ requires std::invocable> #endif - explicit async(Obj &&obj, Fun &&fun, Args&&... args) : detail::async::async_base{std::forward(obj), std::forward(fun), std::forward(args)...} {} + explicit async(Obj &&obj, Fun &&fun, Args&&... args) : async{std::make_shared>()} { + std::invoke(std::forward(fun), std::forward(obj), std::forward(args)..., api_callback); + } /** * @brief Construct an async object wrapping an invokeable object, the call is made immediately by forwarding to std::invoke and can be awaited later to retrieve the result. @@ -442,82 +131,8 @@ class async : private detail::async::async_base { #ifndef _DOXYGEN_ requires std::invocable> #endif - explicit async(Fun &&fun, Args&&... args) : detail::async::async_base{std::forward(fun), std::forward(args)...} {} - -#ifdef _DOXYGEN_ // :) - /** - * @brief Construct an empty async. Using `co_await` on an empty async is undefined behavior. - */ - async() noexcept; - - /** - * @brief Destructor. If any callback is pending it will be aborted. - */ - ~async(); - - /** - * @brief Copy constructor is disabled - */ - async(const async &); - - /** - * @brief Move constructor - * - * NOTE: Despite being marked noexcept, this function uses std::lock_guard which may throw. The implementation assumes this can never happen, hence noexcept. Report it if it does, as that would be a bug. - * - * @remark Using the moved-from async after this function is undefined behavior. - * @param other The async object to move the data from. - */ - async(async &&other) noexcept = default; - - /** - * @brief Copy assignment is disabled - */ - async &operator=(const async &) = delete; - - /** - * @brief Move assignment operator. - * - * NOTE: Despite being marked noexcept, this function uses std::lock_guard which may throw. The implementation assumes this can never happen, hence noexcept. Report it if it does, as that would be a bug. - * - * @remark Using the moved-from async after this function is undefined behavior. - * @param other The async object to move the data from - */ - async &operator=(async &&other) noexcept = default; - - /** - * @brief Check whether or not co_await-ing this would suspend the caller, i.e. if we have the result or not - * - * @return bool Whether we already have the result of the API call or not - */ - [[nodiscard]] bool await_ready() const noexcept; -#endif - - /** - * @brief Suspend the caller until the request completes. - * - * @return On resumption, this expression evaluates to the result object of type R, as a reference. - */ - [[nodiscard]] auto& operator co_await() & noexcept { - return static_cast&>(*this); - } - - /** - * @brief Suspend the caller until the request completes. - * - * @return On resumption, this expression evaluates to the result object of type R, as a const reference. - */ - [[nodiscard]] const auto& operator co_await() const & noexcept { - return static_cast const&>(*this); - } - - /** - * @brief Suspend the caller until the request completes. - * - * @return On resumption, this expression evaluates to the result object of type R, as an rvalue reference. - */ - [[nodiscard]] auto&& operator co_await() && noexcept { - return static_cast&&>(*this); + explicit async(Fun &&fun, Args&&... args) : async{std::make_shared>()} { + std::invoke(std::forward(fun), std::forward(args)..., api_callback); } }; diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h new file mode 100644 index 0000000000..060d4f2ef1 --- /dev/null +++ b/include/dpp/coro/awaitable.h @@ -0,0 +1,736 @@ +/************************************************************************************ + * + * D++, A Lightweight C++ library for Discord + * + * Copyright 2022 Craig Edwards and D++ contributors + * (https://github.com/brainboxdotcc/DPP/graphs/contributors) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ************************************************************************************/ + +#pragma once + +#include + +#include + +namespace dpp { + +struct awaitable_dummy { + int *promise_dummy = nullptr; +}; + +} + +#ifdef DPP_CORO + +#include + +#include +#include +#include +#include +#include +#include + +namespace dpp { + +namespace detail::promise { + +/** + * @brief State of a promise + */ +enum state_flags { + /** + * @brief Promise is empty + */ + sf_none = 0b0000000, + + /** + * @brief Promise has spawned an awaitable + */ + sf_has_awaitable = 0b00000001, + + /** + * @brief Promise is being awaited + */ + sf_awaited = 0b00000010, + + /** + * @brief Promise has a result + */ + sf_ready = 0b00000100, + + /** + * @brief Promise has completed, no more results are expected + */ + sf_done = 0b00001000, + + /** + * @brief Promise was broken - future or promise is gone + */ + sf_broken = 0b0010000 +}; + +template +class promise_base; + +/** + * @brief Empty result from void-returning awaitable + */ +struct empty{}; + +template +void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& result); + +} /* namespace detail::promise */ + +template +requires (requires (Derived t) { detail::co_await_resolve(t); }) +class basic_awaitable { +protected: + template + auto sync_wait_impl(auto&& do_wait) { + using result_type = decltype(detail::co_await_resolve(std::declval()).await_resume()); + using storage_type = std::conditional_t, detail::promise::empty, result_type>; + using variant_type = std::variant; + variant_type result; + std::condition_variable cv; + + detail::promise::spawn_sync_wait_job(static_cast(this), cv, result); + do_wait(cv, result); + if (result.index() == 2) { + std::rethrow_exception(std::get<2>(result)); + } + if constexpr (!Timed) { // no timeout + if constexpr (!std::is_void_v) { + return std::get<1>(result); + } + } else { // timeout + if constexpr (std::is_void_v) { + return result.index() == 1 ? true : false; + } else { + return result.index() == 1 ? std::optional{std::get<1>(result)} : std::nullopt; + } + } + } + +public: + /** + * @brief Blocks this thread and waits for the awaitable to finish. + * + * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. + */ + auto sync_wait() { + return sync_wait_impl([](std::condition_variable &cv, auto&& result) { + std::mutex m{}; + std::unique_lock lock{m}; + cv.wait(lock, [&result] { return result.index() != 0; }); + }); + } + + /** + * @brief Blocks this thread and waits for the awaitable to finish. + * + * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. + * @param duration Maximum duration to wait for + * @retval If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @retval If T is non-void, returns a std::optional for which an absense of value means timed out. + */ + template + auto sync_wait_for(const std::chrono::duration& duration) { + return sync_wait_impl([duration](std::condition_variable &cv, auto&& result) { + std::mutex m{}; + std::unique_lock lock{m}; + cv.wait_for(lock, duration, [&result] { return result.index() != 0; }); + }); + } + + /** + * @brief Blocks this thread and waits for the awaitable to finish. + * + * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. + * @param time Maximum time point to wait for + * @retval If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @retval If T is non-void, returns a std::optional for which an absense of value means timed out. + */ + template + auto sync_wait_until(const std::chrono::time_point &time) { + return sync_wait_impl([time](std::condition_variable &cv, auto&& result) { + std::mutex m{}; + std::unique_lock lock{m}; + cv.wait_until(lock, time, [&result] { return result.index() != 0; }); + }); + } +}; + +/** + * @brief Generic awaitable class, represents a future value that can be co_await-ed on. + * + * Roughly equivalent of std::future for coroutines, with the crucial distinction that the future does not own a reference to a "shared state". + * It holds a non-owning reference to the promise, which must be kept alive for the entire lifetime of the awaitable. + * + * @tparam T Type of the asynchronous value + * @see promise + */ +template +class awaitable : public basic_awaitable> { +protected: + friend class detail::promise::promise_base; + + using shared_state = detail::promise::promise_base; + using state_flags = detail::promise::state_flags; + + /** + * @brief The type of the result produced by this task. + */ + using result_type = T; + + /** + * @brief Non-owning pointer to the promise, which must be kept alive for the entire lifetime of the awaitable. + */ + shared_state *state_ptr = nullptr; + + /** + * @brief Construct from a promise. + * + * @param promise The promise to refer to. + */ + awaitable(shared_state *promise) noexcept : state_ptr{promise} {} + + /** + * @brief Abandons the promise. + * + * Set the promise's state to broken and unlinks this awaitable. + * + * @return uint8_t Flags previously held before setting them to broken + */ + uint8_t abandon(); + /** + * @brief Awaiter returned by co_await. + * + * Contains the await_ready, await_suspend and await_resume functions required by the C++ standard. + * This class is CRTP-like, in that it will refer to an object derived from awaitable. + * + * @tparam Derived Type of reference to refer to the awaitable. + */ + template + struct awaiter { + Derived awaitable_obj; + + /** + * @brief First function called by the standard library when co_await-ing this object. + * + * @throws dpp::logic_exception If the awaitable's valid() would return false. + * @return bool Whether the result is ready, in which case we don't need to suspend + */ + bool await_ready() const; + + /** + * @brief Second function called by the standard library when co_await-ing this object. + * + * @throws dpp::logic_exception If the awaitable's valid() would return false. + * At this point the coroutine frame was allocated and suspended. + * + * @return bool Whether we do need to suspend or not + */ + bool await_suspend(detail::std_coroutine::coroutine_handle<> handle); + + /** + * @brief Third and final function called by the standard library when co_await-ing this object, after resuming. + * + * @throw ? Any exception that occured during the retrieval of the value will be thrown + * @return T The result. + */ + T await_resume(); + }; + +public: + /** + * @brief Construct an empty awaitable. + * + * Such an awaitable must be assigned a promise before it can be awaited. + */ + awaitable() = default; + + /** + * @brief Copy construction is disabled. + */ + awaitable(const awaitable&) = delete; + + /** + * @brief Move from another awaitable. + * + * @param rhs The awaitable to move from, left in an unspecified state after this. + */ + awaitable(awaitable&& rhs) noexcept : state_ptr(std::exchange(rhs.state_ptr, nullptr)) { + } + + /** + * @brief Destructor. + * + * May signal to the promise that it was destroyed. + */ + ~awaitable(); + + /** + * @brief Copy assignment is disabled. + */ + awaitable& operator=(const awaitable&) = delete; + + /** + * @brief Move from another awaitable. + * + * @param rhs The awaitable to move from, left in an unspecified state after this. + * @return *this + */ + awaitable& operator=(awaitable&& rhs) noexcept { + state_ptr = std::exchange(rhs.state_ptr, nullptr); + return *this; + } + + /** + * @brief Check whether this awaitable refers to a valid promise. + * + * @return bool Whether this awaitable refers to a valid promise or not + */ + bool valid() const noexcept; + + /** + * @brief Check whether or not co_await-ing this would suspend the caller, i.e. if we have the result or not + * + * @return bool Whether we already have the result or not + */ + bool await_ready() const; + + /** + * @brief Overload of the co_await operator. + * + * @return Returns an @ref awaiter referencing this awaitable. + */ + template + requires (std::is_base_of_v>) + friend awaiter operator co_await(Derived& obj) noexcept { + return {obj}; + } + + /** + * @brief Overload of the co_await operator. Returns an @ref awaiter referencing this awaitable. + * + * @return Returns an @ref awaiter referencing this awaitable. + */ + template + requires (std::is_base_of_v>) + friend awaiter operator co_await(Derived&& obj) noexcept { + return {std::move(obj)}; + } +}; + +namespace detail::promise { + +template +class promise_base { +protected: + friend class awaitable; + + /** + * @brief Variant representing one of either 3 states of the result value : empty, result, exception. + */ + using storage_type = std::variant, empty, T>, std::exception_ptr>; + + /** + * @brief State of the result value. + * + * @see storage_type + */ + storage_type value = std::monostate{}; + + /** + * @brief State of the awaitable tied to this promise. + */ + std::atomic state = sf_none; + + /** + * @brief Coroutine handle currently awaiting the completion of this promise. + */ + std_coroutine::coroutine_handle<> awaiter = nullptr; + + /** + * @brief Check if the result is empty, throws otherwise. + * + * @throw dpp::logic_exception if the result isn't empty. + */ + void throw_if_not_empty() { + if (value.index() != 0) [[unlikely]] { + throw dpp::logic_exception("cannot set a value on a promise that already has one"); + } + } + + std_coroutine::coroutine_handle<> release_awaiter() { + return std::exchange(awaiter, nullptr); + } + + /** + * @brief Construct a new promise, with empty result. + */ + promise_base() = default; + + /** + * @brief Copy construction is disabled. + */ + promise_base(const promise_base&) = delete; + + /** + * @brief Move construction is disabled. + */ + promise_base(promise_base&& rhs) = delete; + +public: + /** + * @brief Copy assignment is disabled. + */ + promise_base &operator=(const promise_base&) = delete; + + /** + * @brief Move assignment is disabled. + */ + promise_base &operator=(promise_base&& rhs) = delete; + + /** + * @brief Set this promise to an exception and resume any awaiter. + * + * @tparam Notify Whether to resume any awaiter or not. + * @throws dpp::logic_exception if the promise is not empty. + */ + template + void set_exception(std::exception_ptr ptr) { + throw_if_not_empty(); + value.template emplace<2>(std::move(ptr)); + [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready); + if constexpr (Notify) { + if (previous_value & sf_awaited) { + this->awaiter.resume(); + } + } + } + + /** + * @brief Notify a currently awaiting coroutine that the result is ready. + */ + void notify_awaiter() { + if (state.load() & sf_awaited) { + awaiter.resume(); + } + } + + /** + * @brief Get an awaitable object for this promise. + * + * @throws dpp::logic_exception if get_awaitable has already been called on this object. + * @return awaitable An object that can be co_await-ed to retrieve the value of this promise. + */ + awaitable get_awaitable() { + uint8_t previous_flags = state.fetch_or(sf_has_awaitable); + if (previous_flags & sf_has_awaitable) [[unlikely]] { + throw dpp::logic_exception{"an awaitable was already created from this promise"}; + } + return {this}; + } +}; + +/** + * @brief Generic promise class, represents the owning potion of an asynchronous value. + * + * This class is roughly equivalent to std::promise, with the crucial distinction that the promise *IS* the shared state. + * As such, the promise needs to be kept alive for the entire time a value can be retrieved. + * + * @tparam T Type of the asynchronous value + * @see awaitable + */ +template +class promise : public promise_base { +public: + using promise_base::promise_base; + using promise_base::operator=; + + /** + * @brief Construct the result in place by forwarding the arguments, and by default resume any awaiter. + * + * @tparam Notify Whether to resume any awaiter or not. + * @throws dpp::logic_exception if the promise is not empty. + */ + template + requires (std::constructible_from) + void emplace_value(Args&&... args) { + this->throw_if_not_empty(); + try { + this->value.template emplace<1>(std::forward(args)...); + } catch (...) { + this->value.template emplace<2>(std::current_exception()); + } + [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready); + if constexpr (Notify) { + if (previous_value & sf_awaited) { + this->awaiter.resume(); + } + } + } + + /** + * @brief Construct the result by copy, and resume any awaiter. + * + * @tparam Notify Whether to resume any awaiter or not. + * @throws dpp::logic_exception if the promise is not empty. + */ + template + void set_value(const T& v) requires (std::copy_constructible) { + emplace_value(v); + } + + /** + * @brief Construct the result by move, and resume any awaiter. + * + * @tparam Notify Whether to resume any awaiter or not. + * @throws dpp::logic_exception if the promise is not empty. + */ + template + void set_value(T&& v) requires (std::move_constructible) { + emplace_value(std::move(v)); + } +}; + +template <> +class promise : public promise_base { +public: + using promise_base::promise_base; + using promise_base::operator=; + + /** + * @brief Set the promise to completed, and resume any awaiter. + * + * @throws dpp::logic_exception if the promise is not empty. + */ + template + void set_value() { + throw_if_not_empty(); + this->value.emplace<1>(); + [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready); + if constexpr (Notify) { + if (previous_value & sf_awaited) { + this->awaiter.resume(); + } + } + } +}; + +} + +template +using basic_promise = detail::promise::promise; + +/** + * @brief Base class for a promise type. + * + * Contains the base logic for @ref promise, but does not contain the set_value methods. + */ +template +class moveable_promise { + std::unique_ptr> shared_state = std::make_unique>(); + +public: + /** + * @copydoc basic_promise::emplace_value + */ + template + requires (std::constructible_from) + void emplace_value(Args&&... args) { + shared_state->template emplace_value(std::forward(args)...); + } + + /** + * @copydoc basic_promise::set_value(const T&) + */ + template + void set_value(const T& v) requires (std::copy_constructible) { + shared_state->template set_value(v); + } + + /** + * @copydoc basic_promise::set_value(T&&) + */ + template + void set_value(T&& v) requires (std::move_constructible) { + shared_state->template set_value(std::move(v)); + } + + /** + * @copydoc basic_promise::set_value(T&&) + */ + template + void set_exception(std::exception_ptr ptr) { + shared_state->template set_exception(std::move(ptr)); + } + + /** + * @copydoc basic_promise::notify_awaiter + */ + void notify_awaiter() { + shared_state->notify_awaiter(); + } + + /** + * @copydoc basic_promise::get_awaitable + */ + awaitable get_awaitable() { + return shared_state->get_awaitable(); + } +}; + +template <> +class moveable_promise { + std::unique_ptr> shared_state = std::make_unique>(); + +public: + /** + * @copydoc basic_promise::set_value + */ + template + void set_value() { + shared_state->set_value(); + } + + /** + * @copydoc basic_promise::set_exception + */ + template + void set_exception(std::exception_ptr ptr) { + shared_state->set_exception(std::move(ptr)); + } + + /** + * @copydoc basic_promise::notify_awaiter + */ + void notify_awaiter() { + shared_state->notify_awaiter(); + } + + /** + * @copydoc basic_promise::get_awaitable + */ + awaitable get_awaitable() { + return shared_state->get_awaitable(); + } +}; + +template +using promise = moveable_promise; + +template +auto awaitable::abandon() -> uint8_t { + auto previous_state = state_ptr->state.fetch_or(state_flags::sf_broken); + state_ptr = nullptr; + return previous_state; +} + +template +awaitable::~awaitable() { + if (state_ptr) { + state_ptr->state.fetch_or(state_flags::sf_broken); + } +} + +template +bool awaitable::valid() const noexcept { + return state_ptr != nullptr; +} + +template +bool awaitable::await_ready() const { + if (!this->valid()) { + throw dpp::logic_exception("cannot co_await an empty awaitable"); + } + uint8_t state = this->state_ptr->state.load(std::memory_order_relaxed); + return state & detail::promise::sf_ready; +} + +template +template +bool awaitable::awaiter::await_suspend(detail::std_coroutine::coroutine_handle<> handle) { + auto &promise = *awaitable_obj.state_ptr; + + promise.awaiter = handle; + auto previous_flags = promise.state.fetch_or(detail::promise::sf_awaited); + if (previous_flags & detail::promise::sf_awaited) { + throw dpp::logic_exception("awaitable is already being awaited"); + } + return !(previous_flags & detail::promise::sf_ready); +} + +template +template +T awaitable::awaiter::await_resume() { + auto &promise = *std::exchange(awaitable_obj.state_ptr, nullptr); + + promise.state.fetch_and(~detail::promise::sf_awaited); + if (std::holds_alternative(promise.value)) { + std::rethrow_exception(std::get<2>(promise.value)); + } + if constexpr (!std::is_void_v) { + return std::get<1>(std::move(promise.value)); + } else { + return; + } +} + + + +template +template +bool awaitable::awaiter::await_ready() const { + return static_cast(awaitable_obj).await_ready(); +} + +} + +#include + +namespace dpp { + +namespace detail::promise { + +template +using result_t = std::variant, empty, T>, std::exception_ptr>; + +template +void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& result) { + [](auto* awaitable_, std::condition_variable &cv_, auto&& result_) -> dpp::job { + try { + if constexpr (std::is_void_v) { + co_await *awaitable_; + result_.template emplace<1>(); + } else { + result_.template emplace<1>(co_await *awaitable_); + } + } catch (...) { + result_.template emplace<2>(std::current_exception()); + } + cv_.notify_all(); + }(awaitable, cv, std::forward(result)); +} + +} + +} + +#endif /* DPP_CORO */ diff --git a/include/dpp/coro/coroutine.h b/include/dpp/coro/coroutine.h index 18fcac7f1c..4be9021663 100644 --- a/include/dpp/coro/coroutine.h +++ b/include/dpp/coro/coroutine.h @@ -55,188 +55,86 @@ template */ using handle_t = std_coroutine::coroutine_handle>; +} // namespace coroutine + +} // namespace detail + /** - * @brief Base class of dpp::coroutine. + * @class coroutine coroutine.h coro/coroutine.h + * @brief Base type for a coroutine, starts on co_await. * - * @warning This class should not be used directly by a user, use dpp::coroutine instead. - * @note This class contains all the functions used internally by co_await. It is intentionally opaque and a private base of dpp::coroutine so a user cannot call await_suspend and await_resume directly. + * @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. + * Please report any to GitHub Issues or to our Discord Server. + * @warning - Using co_await on this object more than once is undefined behavior. + * @tparam R Return type of the coroutine. Can be void, or a complete object that supports move construction and move assignment. */ template -class coroutine_base { -protected: +class coroutine : public basic_awaitable> { /** * @brief Promise has friend access for the constructor */ - friend struct promise_t; + friend struct detail::coroutine::promise_t; /** * @brief Coroutine handle. */ detail::coroutine::handle_t handle{nullptr}; -private: /** * @brief Construct from a handle. Internal use only. */ - coroutine_base(detail::coroutine::handle_t h) : handle{h} {} - -public: - /** - * @brief Default constructor, creates an empty coroutine. - */ - coroutine_base() = default; - - /** - * @brief Copy constructor is disabled - */ - coroutine_base(const coroutine_base &) = delete; - - /** - * @brief Move constructor, grabs another coroutine's handle - * - * @param other Coroutine to move the handle from - */ - coroutine_base(coroutine_base &&other) noexcept : handle(std::exchange(other.handle, nullptr)) {} - - /** - * @brief Destructor, destroys the handle. - */ - ~coroutine_base() { - if (handle) { - handle.destroy(); - } - } - - /** - * @brief Copy assignment is disabled - */ - coroutine_base &operator=(const coroutine_base &) = delete; - - /** - * @brief Move assignment, grabs another coroutine's handle - * - * @param other Coroutine to move the handle from - */ - coroutine_base &operator=(coroutine_base &&other) noexcept { - handle = std::exchange(other.handle, nullptr); - return *this; - } - - /** - * @brief First function called by the standard library when the coroutine is co_await-ed. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @throws invalid_operation_exception if the coroutine is empty or finished. - * @return bool Whether the coroutine is done - */ - [[nodiscard]] bool await_ready() const { - if (!handle) { - throw dpp::logic_exception("cannot co_await an empty coroutine"); - } - return handle.done(); - } + coroutine(detail::coroutine::handle_t h) : handle{h} {} - /** - * @brief Second function called by the standard library when the coroutine is co_await-ed. - * - * Stores the calling coroutine in the promise to resume when this coroutine suspends. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @param caller The calling coroutine, now suspended - */ - template - [[nodiscard]] handle_t await_suspend(detail::std_coroutine::coroutine_handle caller) noexcept { - handle.promise().parent = caller; - return handle; - } - - /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @throw Throws any exception thrown or uncaught by the coroutine - * @return R The result of the coroutine. It is given to the caller as a result to `co_await` - */ - decltype(auto) await_resume() & { - return static_cast &>(*this).await_resume_impl(); - } + struct awaiter { + coroutine &coro; - /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @throw Throws any exception thrown or uncaught by the coroutine - * @return R The result of the coroutine. It is given to the caller as a result to `co_await` - */ - [[nodiscard]] decltype(auto) await_resume() const & { - return static_cast const&>(*this).await_resume_impl(); - } - - /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @throw Throws any exception thrown or uncaught by the coroutine - * @return R The result of the coroutine. It is given to the caller as a result to `co_await` - */ - [[nodiscard]] decltype(auto) await_resume() && { - return static_cast &&>(*this).await_resume_impl(); - } -}; - -} // namespace coroutine - -} // namespace detail - -/** - * @class coroutine coroutine.h coro/coroutine.h - * @brief Base type for a coroutine, starts on co_await. - * - * @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. - * Please report any to GitHub Issues or to our Discord Server. - * @warning - Using co_await on this object more than once is undefined behavior. - * @tparam R Return type of the coroutine. Can be void, or a complete object that supports move construction and move assignment. - */ -template -class coroutine : private detail::coroutine::coroutine_base { - /** - * @brief Internal use only base class containing common logic between coroutine and coroutine. It also serves to prevent await_suspend and await_resume from being used directly. - * - * @warning For internal use only, do not use. - * @see operator co_await() - */ - friend class detail::coroutine::coroutine_base; - - [[nodiscard]] R& await_resume_impl() & { - detail::coroutine::promise_t &promise = this->handle.promise(); - if (promise.exception) { - std::rethrow_exception(promise.exception); + /** + * @brief First function called by the standard library when the coroutine is co_await-ed. + * + * @remark Do not call this manually, use the co_await keyword instead. + * @throws invalid_operation_exception if the coroutine is empty or finished. + * @return bool Whether the coroutine is done + */ + [[nodiscard]] bool await_ready() const { + if (!coro.handle) { + throw dpp::logic_exception("cannot co_await an empty coroutine"); + } + return coro.handle.done(); } - return *promise.result; - } - [[nodiscard]] const R& await_resume_impl() const & { - detail::coroutine::promise_t &promise = this->handle.promise(); - if (promise.exception) { - std::rethrow_exception(promise.exception); + /** + * @brief Second function called by the standard library when the coroutine is co_await-ed. + * + * Stores the calling coroutine in the promise to resume when this coroutine suspends. + * + * @remark Do not call this manually, use the co_await keyword instead. + * @param caller The calling coroutine, now suspended + */ + template + [[nodiscard]] detail::coroutine::handle_t await_suspend(detail::std_coroutine::coroutine_handle caller) noexcept { + coro.handle.promise().parent = caller; + return coro.handle; } - return *promise.result; - } - [[nodiscard]] R&& await_resume_impl() && { - detail::coroutine::promise_t &promise = this->handle.promise(); - if (promise.exception) { - std::rethrow_exception(promise.exception); + R await_resume() { + detail::coroutine::promise_t &promise = coro.handle.promise(); + if (promise.exception) { + std::rethrow_exception(promise.exception); + } + if constexpr (!std::is_void_v) { + return *std::exchange(promise.result, std::nullopt); + } else { + return; // unnecessary but makes lsp happy + } } - return *std::move(promise.result); - } + }; public: /** * @brief The type of the result produced by this coroutine. */ using result_type = R; -#ifdef _DOXYGEN_ // :)))) + /** * @brief Default constructor, creates an empty coroutine. */ @@ -252,12 +150,16 @@ class coroutine : private detail::coroutine::coroutine_base { * * @param other Coroutine to move the handle from */ - coroutine(coroutine &&other) noexcept; + coroutine(coroutine &&other) noexcept : handle(std::exchange(other.handle, nullptr)) {} /** * @brief Destructor, destroys the handle. */ - ~coroutine(); + ~coroutine() { + if (handle) { + handle.destroy(); + } + } /** * @brief Copy assignment is disabled @@ -269,111 +171,15 @@ class coroutine : private detail::coroutine::coroutine_base { * * @param other Coroutine to move the handle from */ - coroutine &operator=(coroutine &&other) noexcept; - - /** - * @brief First function called by the standard library when the coroutine is co_await-ed. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @throws invalid_operation_exception if the coroutine is empty or finished. - * @return bool Whether the coroutine is done - */ - [[nodiscard]] bool await_ready() const; -#else - using detail::coroutine::coroutine_base::coroutine_base; // use coroutine_base's constructors - using detail::coroutine::coroutine_base::operator=; // use coroutine_base's assignment operators - using detail::coroutine::coroutine_base::await_ready; // expose await_ready as public -#endif - - /** - * @brief Suspend the caller until the coroutine completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as a reference. - */ - [[nodiscard]] auto& operator co_await() & noexcept { - return static_cast&>(*this); - } - - /** - * @brief Suspend the caller until the coroutine completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as a const reference. - */ - [[nodiscard]] const auto& operator co_await() const & noexcept { - return static_cast const&>(*this); - } - - /** - * @brief Suspend the caller until the coroutine completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as an rvalue reference. - */ - [[nodiscard]] auto&& operator co_await() && noexcept { - return static_cast&&>(*this); - } -}; - -#ifndef _DOXYGEN_ // don't generate this on doxygen because `using` doesn't work and 2 copies of coroutine_base's docs is enough -/** - * @brief Base type for a coroutine, starts on co_await. - * - * @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to GitHub issues or to the D++ Discord server. - * @warning - Using co_await on this object more than once is undefined behavior. - * @tparam R Return type of the coroutine. Can be void, or a complete object that supports move construction and move assignment. - */ -template <> -class coroutine : private detail::coroutine::coroutine_base { - /** - * @brief Base class has friend access for CRTP downcast - */ - friend class detail::coroutine::coroutine_base; - - void await_resume_impl() const; - -public: - using detail::coroutine::coroutine_base::coroutine_base; // use coroutine_base's constructors - using detail::coroutine::coroutine_base::operator=; // use coroutine_base's assignment operators - using detail::coroutine::coroutine_base::await_ready; // expose await_ready as public - - /** - * @brief The type of the result produced by this coroutine. - */ - using result_type = void; - - /** - * @brief Suspend the current coroutine until the coroutine completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as a reference. - */ - [[nodiscard]] auto& operator co_await() & noexcept { - return static_cast&>(*this); - } - - /** - * @brief Suspend the current coroutine until the coroutine completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as a const reference. - */ - [[nodiscard]] const auto& operator co_await() const & noexcept { - return static_cast const &>(*this); + coroutine &operator=(coroutine &&other) noexcept { + handle = std::exchange(other.handle, nullptr); + return *this; } - - /** - * @brief Suspend the current coroutine until the coroutine completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as an rvalue reference. - */ - [[nodiscard]] auto&& operator co_await() && noexcept { - return static_cast&&>(*this); + + [[nodiscard]] auto operator co_await() { + return awaiter{*this}; } }; -#endif /* _DOXYGEN_ */ namespace detail::coroutine { template @@ -574,14 +380,6 @@ namespace detail::coroutine { } // namespace detail -#ifndef _DOXYGEN_ -inline void coroutine::await_resume_impl() const { - if (handle.promise().exception) { - std::rethrow_exception(handle.promise().exception); - } -} -#endif /* _DOXYGEN_ */ - DPP_CHECK_ABI_COMPAT(coroutine, coroutine_dummy) DPP_CHECK_ABI_COMPAT(coroutine, coroutine_dummy) diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index 74c9f981d7..a7c251224a 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -21,10 +21,11 @@ #pragma once #include +#include namespace dpp { -struct task_dummy { +struct task_dummy : awaitable_dummy { int* handle_dummy = nullptr; }; @@ -32,7 +33,7 @@ struct task_dummy { #ifdef DPP_CORO -#include "coro.h" +#include #include #include @@ -51,28 +52,6 @@ namespace detail { /* Internal cogwheels for dpp::task */ namespace task { -/** - * @brief State of a task - */ -enum class state_t { - /** - * @brief Task was started but never co_await-ed - */ - started, - /** - * @brief Task was co_await-ed and is pending completion - */ - awaited, - /** - * @brief Task is completed - */ - done, - /** - * @brief Task is still running but the actual dpp::task object is destroyed - */ - dangling -}; - /** * @brief A @ref dpp::task "task"'s promise_t type, with special logic for handling nested tasks. * @@ -97,176 +76,6 @@ struct final_awaiter; template using handle_t = std_coroutine::coroutine_handle>; -/** - * @brief Base class of @ref dpp::task. - * - * @warning This class should not be used directly by a user, use @ref dpp::task instead. - * @note This class contains all the functions used internally by co_await. It is intentionally opaque and a private base of @ref dpp::task so a user cannot call await_suspend() and await_resume() directly. - */ -template -class task_base { -protected: - /** - * @brief The coroutine handle of this task. - */ - handle_t handle; - - /** - * @brief Promise type of this coroutine. For internal use only, do not use. - */ - friend struct promise_t; - -private: - /** - * @brief Construct from a coroutine handle. Internal use only - */ - explicit task_base(handle_t handle_) : handle(handle_) {} - -public: - /** - * @brief Default constructor, creates a task not bound to a coroutine. - */ - task_base() = default; - - /** - * @brief Copy constructor is disabled - */ - task_base(const task_base &) = delete; - - /** - * @brief Move constructor, grabs another task's coroutine handle - * - * @param other Task to move the handle from - */ - task_base(task_base &&other) noexcept : handle(std::exchange(other.handle, nullptr)) {} - - /** - * @brief Destructor. - * - * Destroys the handle. - * @warning The coroutine must be finished before this is called, otherwise it runs the risk of being resumed after it is destroyed, resuming in use-after-free undefined behavior. - */ - ~task_base() { - if (handle) { - promise_t &promise = handle.promise(); - state_t previous_state = promise.state.exchange(state_t::dangling); - - if (previous_state == state_t::done) { - handle.destroy(); - } - else { - cancel(); - } - } - } - - /** - * @brief Copy assignment is disabled - */ - task_base &operator=(const task_base &) = delete; - - /** - * @brief Move assignment, grabs another task's coroutine handle - * - * @param other Task to move the handle from - */ - task_base &operator=(task_base &&other) noexcept { - handle = std::exchange(other.handle, nullptr); - return (*this); - } - - /** - * @brief Check whether or not a call to co_await will suspend the caller. - * - * This function is called by the standard library as a first step when using co_await. If it returns true then the caller is not suspended. - * @throws logic_exception if the task is empty. - * @return bool Whether not to suspend the caller or not - */ - [[nodiscard]] bool await_ready() const { - if (!handle) { - throw dpp::logic_exception{"cannot co_await an empty task"}; - } - return handle.promise().state.load() == state_t::done; - } - - /** - * @brief Second function called by the standard library when the task is co_await-ed, if await_ready returned false. - * - * Stores the calling coroutine in the promise to resume when this task suspends. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @param caller The calling coroutine, now suspended - * @return bool Whether to suspend the caller or not - */ - [[nodiscard]] bool await_suspend(std_coroutine::coroutine_handle<> caller) noexcept { - promise_t &my_promise = handle.promise(); - auto previous_state = state_t::started; - - my_promise.parent = caller; - // Replace `sent` state with `awaited` ; if that fails, the only logical option is the state was `done`, in which case return false to resume - if (!handle.promise().state.compare_exchange_strong(previous_state, state_t::awaited) && previous_state == state_t::done) { - return false; - } - return true; - } - - /** - * @brief Function to check if the task has finished its execution entirely - * - * @return bool Whether the task is finished. - */ - [[nodiscard]] bool done() const noexcept { - return handle && handle.promise().state.load(std::memory_order_relaxed) == state_t::done; - } - - /** - * @brief Cancel the task, it will stop the next time it uses co_await. On co_await-ing this task, throws dpp::task_cancelled_exception. - * - * @return *this - */ - dpp::task& cancel() & noexcept { - handle.promise().cancelled.exchange(true, std::memory_order_relaxed); - return static_cast &>(*this); - } - - /** - * @brief Cancel the task, it will stop the next time it uses co_await. On co_await-ing this task, throws dpp::task_cancelled_exception. - * - * @return *this - */ - dpp::task&& cancel() && noexcept { - handle.promise().cancelled.exchange(true, std::memory_order_relaxed); - return static_cast &&>(*this); - } - - /** - * @brief Function called by the standard library when resuming. - * - * @return Return value of the coroutine, handed to the caller of co_await. - */ - decltype(auto) await_resume() & { - return static_cast &>(*this).await_resume_impl(); - } - - /** - * @brief Function called by the standard library when resuming. - * - * @return Return value of the coroutine, handed to the caller of co_await. - */ - decltype(auto) await_resume() const & { - return static_cast &>(*this).await_resume_impl(); - } - - /** - * @brief Function called by the standard library when resuming. - * - * @return Return value of the coroutine, handed to the caller of co_await. - */ - decltype(auto) await_resume() && { - return static_cast &&>(*this).await_resume_impl(); - } -}; - } // namespace task } // namespace detail @@ -283,63 +92,21 @@ template #ifndef _DOXYGEN_ requires (!std::is_reference_v) #endif -class task : private detail::task::task_base { - /** - * @brief Internal use only base class containing common logic between task and task. It also serves to prevent await_suspend and await_resume from being used directly. - * - * @warning For internal use only, do not use. - * @see operator co_await() - */ - friend class detail::task::task_base; +class task : public awaitable { + friend struct detail::task::promise_t; - /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @throw Throws any exception thrown or uncaught by the coroutine - * @return The result of the coroutine. This is returned to the awaiter as the result of co_await - */ - R& await_resume_impl() & { - detail::task::promise_t &promise = this->handle.promise(); - if (promise.exception) { - std::rethrow_exception(promise.exception); - } - return *reinterpret_cast(promise.result_storage.data()); - } + using handle_t = detail::task::handle_t; + using state_flags = detail::promise::state_flags; - /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @throw Throws any exception thrown or uncaught by the coroutine - * @return The result of the coroutine. This is returned to the awaiter as the result of co_await - */ - const R& await_resume_impl() const & { - detail::task::promise_t &promise = this->handle.promise(); - if (promise.exception) { - std::rethrow_exception(promise.exception); - } - return *reinterpret_cast(promise.result_storage.data()); - } + handle_t handle{}; +protected: /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @throw Throws any exception thrown or uncaught by the coroutine - * @return The result of the coroutine. This is returned to the awaiter as the result of co_await + * @brief Construct from a coroutine handle. Internal use only */ - R&& await_resume_impl() && { - detail::task::promise_t &promise = this->handle.promise(); - if (promise.exception) { - std::rethrow_exception(promise.exception); - } - return *reinterpret_cast(promise.result_storage.data()); - } + explicit task(handle_t handle_) : awaitable(&handle_.promise()), handle(handle_) {} public: - /** - * @brief The type of the result produced by this task. - */ - using result_type = R; -#ifdef _DOXYGEN_ // :) /** * @brief Default constructor, creates a task not bound to a coroutine. */ @@ -355,15 +122,7 @@ class task : private detail::task::task_base { * * @param other Task to move the handle from */ - task(task &&other) noexcept; - - /** - * @brief Destructor. - * - * Destroys the handle. - * @warning The coroutine must be finished before this is called, otherwise it runs the risk of being resumed after it is destroyed, resuming in use-after-free undefined behavior. - */ - ~task(); + task(task &&other) noexcept : awaitable(std::move(other)), handle(std::exchange(other.handle, nullptr)) {} /** * @brief Copy assignment is disabled @@ -375,136 +134,56 @@ class task : private detail::task::task_base { * * @param other Task to move the handle from */ - task &operator=(task &&other) noexcept; - - /** - * @brief Function to check if the task has finished its execution entirely - * - * @return bool Whether the task is finished. - */ - [[nodiscard]] bool done() const noexcept; - - /** - * @brief Cancel the task, it will stop the next time it uses co_await. On co_await-ing this task, throws dpp::task_cancelled_exception. - */ - dpp::task& cancel() & noexcept; - - /** - * @brief Check whether or not a call to co_await will suspend the caller. - * - * This function is called by the standard library as a first step when using co_await. If it returns true then the caller is not suspended. - * @throws logic_exception if the task is empty. - * @return bool Whether not to suspend the caller or not - */ - [[nodiscard]] bool await_ready() const; -#else - using detail::task::task_base::task_base; // use task_base's constructors - using detail::task::task_base::operator=; // use task_base's assignment operators - using detail::task::task_base::done; // expose done() as public - using detail::task::task_base::cancel; // expose cancel() as public - using detail::task::task_base::await_ready; // expose await_ready as public -#endif - - /** - * @brief Suspend the current coroutine until the task completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as a reference. - */ - [[nodiscard]] auto& operator co_await() & noexcept { - return static_cast&>(*this); + task &operator=(task &&other) noexcept { + awaitable::operator=(std::move(other)); + handle = std::exchange(other.handle, nullptr); + return *this; } /** - * @brief Suspend the current coroutine until the task completes. + * @brief Destructor. * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as a const reference. + * Destroys the handle. If the task is still running, it will be cancelled. */ - [[nodiscard]] const auto& operator co_await() const & noexcept { - return static_cast&>(*this); + ~task() { + if (handle && this->valid()) { + if (this->abandon() & state_flags::sf_done) { + handle.destroy(); + } else { + cancel(); + } + } } /** - * @brief Suspend the current coroutine until the task completes. - * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, this expression evaluates to the result object of type R, as an rvalue reference. - */ - [[nodiscard]] auto&& operator co_await() && noexcept { - return static_cast&&>(*this); - } -}; - -#ifndef _DOXYGEN_ // don't generate this on doxygen because `using` doesn't work and 2 copies of coroutine_base's docs is enough -/** - * @brief A coroutine task. It starts immediately on construction and can be co_await-ed, making it perfect for parallel coroutines returning a value. - * - * Can be used in conjunction with coroutine events via dpp::event_router_t::co_attach, or on its own. - * - * @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to GitHub issues or to the D++ Discord server. - * @tparam R Return type of the coroutine. Cannot be a reference, can be void. - */ -template <> -class task : private detail::task::task_base { - /** - * @brief Private base class containing common logic between task and task. It also serves to prevent await_suspend and await_resume from being used directly. - * - * @see operator co_await() - */ - friend class detail::task::task_base; - - /** - * @brief Function called by the standard library when the coroutine is resumed. - * - * @remark Do not call this manually, use the co_await keyword instead. - * @throw Throws any exception thrown or uncaught by the coroutine - */ - void await_resume_impl() const; - -public: - using detail::task::task_base::task_base; // use task_base's constructors - using detail::task::task_base::operator=; // use task_base's assignment operators - using detail::task::task_base::done; // expose done() as public - using detail::task::task_base::cancel; // expose cancel() as public - using detail::task::task_base::await_ready; // expose await_ready as public - - /** - * @brief The type of the result produced by this task - */ - using result_type = void; - - /** - * @brief Suspend the current coroutine until the task completes. + * @brief Function to check if the task has finished its execution entirely * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, returns a reference to the contained result. + * @return bool Whether the task is finished. */ - auto& operator co_await() & { - return static_cast&>(*this); + [[nodiscard]] bool done() const noexcept { + return handle && (!this->valid() || handle.promise().state.load(std::memory_order_relaxed) == state_flags::sf_done); } /** - * @brief Suspend the current coroutine until the task completes. + * @brief Cancel the task, it will stop the next time it uses co_await. On co_await-ing this task, throws dpp::task_cancelled_exception. * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, returns a const reference to the contained result. + * @return *this */ - const auto& operator co_await() const & { - return static_cast&>(*this); + task& cancel() & noexcept { + handle.promise().cancelled.exchange(true, std::memory_order_relaxed); + return *this; } /** - * @brief Suspend the current coroutine until the task completes. + * @brief Cancel the task, it will stop the next time it uses co_await. On co_await-ing this task, throws dpp::task_cancelled_exception. * - * @throw On resumption, any exception thrown by the coroutine is propagated to the caller. - * @return On resumption, returns a reference to the contained result. + * @return *this */ - auto&& operator co_await() && { - return static_cast&&>(*this); + task&& cancel() && noexcept { + handle.promise().cancelled.exchange(true, std::memory_order_relaxed); + return *this; } }; -#endif /* _DOXYGEN_ */ namespace detail::task { /** @@ -536,29 +215,13 @@ struct final_awaiter { /** * @brief Base implementation of task::promise_t, without the logic that would depend on the return type. Meant to be inherited from */ -struct promise_base { - /** - * @brief State of the task, used to keep track of lifetime and status - */ - std::atomic state = state_t::started; - +template +struct promise_base : basic_promise { /** * @brief Whether the task is cancelled or not. */ std::atomic cancelled = false; - /** - * @brief Parent coroutine to return to for nested coroutines. - */ - detail::std_coroutine::coroutine_handle<> parent = nullptr; - - /** - * @brief Exception ptr if any was thrown during the coroutine - * - * @see ::await_resume_impl() const { - if (handle.promise().exception) { - std::rethrow_exception(handle.promise().exception); - } -} -#endif /* _DOXYGEN_ */ - DPP_CHECK_ABI_COMPAT(task, task_dummy) DPP_CHECK_ABI_COMPAT(task, task_dummy) diff --git a/src/unittest/coro.cpp b/src/unittest/coro.cpp index c5047a134c..ea35e919ca 100644 --- a/src/unittest/coro.cpp +++ b/src/unittest/coro.cpp @@ -365,7 +365,110 @@ dpp::job async_test() { set_status(test, ts_success); } catch (const std::exception &e) { /* no exception should be caught here */ - set_status(test, ts_failed, "unknown exception thrown"); + set_status(test, ts_failed, std::string{"unknown exception thrown: "} + e.what()); + } +} + +dpp::job coro_awaitable_test() { + try { + { + dpp::promise test; + + test.set_value(42); + if (int res = co_await test.get_awaitable(); res != 42) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "could not retrieve value set before co_await"); + } + } + { + dpp::promise test; + + test.set_value(420); + if (int res = test.get_awaitable().sync_wait(); res != 420) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "could not retrieve value set before sync_wait"); + } + } + { + dpp::promise test; + dpp::awaitable awaitable; + + awaitable = test.get_awaitable(); + test.set_value(420); + if (std::optional res = awaitable.sync_wait_for(std::chrono::seconds(5)); !res || *res != 420) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "could not retrieve value set before sync_wait_for"); + } + } + { + dpp::promise test; + dpp::awaitable awaitable{test.get_awaitable()}; + + if (bool res = awaitable.sync_wait_for(std::chrono::seconds(5)); res) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "could not retrieve time out with sync_wait_for"); + } + } + { + dpp::promise test; + dpp::awaitable awaitable{test.get_awaitable()}; + std::thread th{[p = std::move(test)]() mutable { + std::this_thread::sleep_for(std::chrono::seconds(2)); + p.set_value(69); + }}; + th.detach(); + if (std::optional res = awaitable.sync_wait_for(std::chrono::seconds(5)); !res || *res != 69) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "could not retrieve value set after sync_wait_for"); + } + } + { + dpp::promise test; + dpp::awaitable awaitable{test.get_awaitable()}; + std::thread th{[p = std::move(test)]() mutable { + std::this_thread::sleep_for(std::chrono::seconds(2)); + p.set_value(69420); + }}; + th.detach(); + if (int res = co_await awaitable; res != 69420) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "could not retrieve value set after co_await"); + } + } + { + dpp::promise test; + dpp::awaitable awaitable{test.get_awaitable()}; + std::thread th{[p = std::move(test)]() mutable { + std::this_thread::sleep_for(std::chrono::seconds(2)); + p.set_exception(std::make_exception_ptr(dpp::voice_exception("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))); + }}; + th.detach(); + bool success = false; + try { + co_await awaitable; + } catch (const dpp::voice_exception &) { + success = true; + } + if (!success) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "retrieval of an exception with co_await failed"); + } + } + { + dpp::promise test; + dpp::awaitable awaitable{test.get_awaitable()}; + std::thread th{[p = std::move(test)]() mutable { + std::this_thread::sleep_for(std::chrono::seconds(2)); + p.set_exception(std::make_exception_ptr(dpp::voice_exception("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))); + }}; + th.detach(); + bool success = false; + try { + awaitable.sync_wait(); + } catch (const dpp::voice_exception &) { + success = true; + } + if (!success) { + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, "retrieval of an exception with co_await failed"); + } + } + set_status(CORO_AWAITABLE_OFFLINE, ts_success); + } catch (const std::exception &e) { + // no exception should reach this point + set_status(CORO_AWAITABLE_OFFLINE, ts_failed, std::string{"unknown exception thrown: "} + e.what()); } } @@ -378,6 +481,9 @@ void coro_offline_tests() std::fill(job_data.begin(), job_data.end(), -1); job_offline_test(); + start_test(CORO_AWAITABLE_OFFLINE); + coro_awaitable_test(); + start_test(CORO_TASK_OFFLINE); std::fill(task_data.begin(), task_data.end(), -1); []() -> dpp::job { @@ -388,7 +494,7 @@ void coro_offline_tests() } catch (const test_exception<0> &) { // exception thrown at the end of the task test set_status(CORO_TASK_OFFLINE, ts_success); } catch (const std::exception &e) { // anything else should not escape - set_status(CORO_TASK_OFFLINE, ts_failed, "unknown exception thrown"); + set_status(CORO_TASK_OFFLINE, ts_failed, std::string{"unknown exception thrown: "} + e.what()); } }(); @@ -401,7 +507,7 @@ void coro_offline_tests() } catch (const test_exception<0> &) { set_status(CORO_COROUTINE_OFFLINE, ts_success); } catch (const std::exception &e) { // anything else should not escape - set_status(CORO_COROUTINE_OFFLINE, ts_failed, "unknown exception thrown"); + set_status(CORO_COROUTINE_OFFLINE, ts_failed, std::string{"unknown exception thrown: "} + e.what()); } }(); diff --git a/src/unittest/test.h b/src/unittest/test.h index 88840728a4..b9b6fa7d2d 100644 --- a/src/unittest/test.h +++ b/src/unittest/test.h @@ -256,6 +256,7 @@ DPP_TEST(THREAD_MESSAGE_REACT_ADD_EVENT, "cluster::on_reaction_add in thread", t DPP_TEST(THREAD_MESSAGE_REACT_REMOVE_EVENT, "cluster::on_reaction_remove in thread", tf_online | tf_extended); DPP_TEST(CORO_JOB_OFFLINE, "coro: offline job", tf_offline | tf_coro); +DPP_TEST(CORO_AWAITABLE_OFFLINE, "coro: offline promise & awaitable", tf_offline | tf_coro); DPP_TEST(CORO_COROUTINE_OFFLINE, "coro: offline coroutine", tf_offline | tf_coro); DPP_TEST(CORO_TASK_OFFLINE, "coro: offline task", tf_offline | tf_coro); DPP_TEST(CORO_ASYNC_OFFLINE, "coro: offline async", tf_offline | tf_coro); From 79800dbb4e81b870593a83e279bbef9d2721a633 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Fri, 2 Feb 2024 01:53:18 -0500 Subject: [PATCH 02/17] fix(coro): fix includes, improve NOMINMAX define --- include/dpp/coro/coroutine.h | 3 ++- include/dpp/export.h | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/include/dpp/coro/coroutine.h b/include/dpp/coro/coroutine.h index 4be9021663..dfe664e14a 100644 --- a/include/dpp/coro/coroutine.h +++ b/include/dpp/coro/coroutine.h @@ -32,7 +32,8 @@ struct coroutine_dummy { #ifdef DPP_CORO -#include "coro.h" +#include +#include #include #include diff --git a/include/dpp/export.h b/include/dpp/export.h index 24d6bfe81a..e293aafccc 100644 --- a/include/dpp/export.h +++ b/include/dpp/export.h @@ -118,7 +118,9 @@ extern bool DPP_EXPORT validate_configuration(); #ifndef _WIN32 #define SOCKET int #else - #define NOMINMAX + #ifndef NOMINMAX + #define NOMINMAX + #endif #include #endif From 69dd90216db8a3f4a987ba471e7460fdd886f548 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 6 Jul 2024 01:05:58 -0400 Subject: [PATCH 03/17] improv(coro): use std::memory_order --- include/dpp/coro/awaitable.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 060d4f2ef1..1859e996fd 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -417,7 +417,7 @@ class promise_base { void set_exception(std::exception_ptr ptr) { throw_if_not_empty(); value.template emplace<2>(std::move(ptr)); - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready); + [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { if (previous_value & sf_awaited) { this->awaiter.resume(); @@ -429,7 +429,7 @@ class promise_base { * @brief Notify a currently awaiting coroutine that the result is ready. */ void notify_awaiter() { - if (state.load() & sf_awaited) { + if (state.load(std::memory_order::acquire) & sf_awaited) { awaiter.resume(); } } @@ -441,7 +441,7 @@ class promise_base { * @return awaitable An object that can be co_await-ed to retrieve the value of this promise. */ awaitable get_awaitable() { - uint8_t previous_flags = state.fetch_or(sf_has_awaitable); + uint8_t previous_flags = state.fetch_or(sf_has_awaitable, std::memory_order::relaxed); if (previous_flags & sf_has_awaitable) [[unlikely]] { throw dpp::logic_exception{"an awaitable was already created from this promise"}; } @@ -479,7 +479,7 @@ class promise : public promise_base { } catch (...) { this->value.template emplace<2>(std::current_exception()); } - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready); + [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { if (previous_value & sf_awaited) { this->awaiter.resume(); @@ -525,7 +525,7 @@ class promise : public promise_base { void set_value() { throw_if_not_empty(); this->value.emplace<1>(); - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready); + [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { if (previous_value & sf_awaited) { this->awaiter.resume(); @@ -638,7 +638,7 @@ using promise = moveable_promise; template auto awaitable::abandon() -> uint8_t { - auto previous_state = state_ptr->state.fetch_or(state_flags::sf_broken); + auto previous_state = state_ptr->state.fetch_or(state_flags::sf_broken, std::memory_order::acq_rel); state_ptr = nullptr; return previous_state; } @@ -646,7 +646,7 @@ auto awaitable::abandon() -> uint8_t { template awaitable::~awaitable() { if (state_ptr) { - state_ptr->state.fetch_or(state_flags::sf_broken); + state_ptr->state.fetch_or(state_flags::sf_broken, std::memory_order::acq_rel); } } @@ -660,7 +660,7 @@ bool awaitable::await_ready() const { if (!this->valid()) { throw dpp::logic_exception("cannot co_await an empty awaitable"); } - uint8_t state = this->state_ptr->state.load(std::memory_order_relaxed); + uint8_t state = this->state_ptr->state.load(std::memory_order::relaxed); return state & detail::promise::sf_ready; } @@ -670,7 +670,7 @@ bool awaitable::awaiter::await_suspend(detail::std_coroutine::corout auto &promise = *awaitable_obj.state_ptr; promise.awaiter = handle; - auto previous_flags = promise.state.fetch_or(detail::promise::sf_awaited); + auto previous_flags = promise.state.fetch_or(detail::promise::sf_awaited, std::memory_order::relaxed); if (previous_flags & detail::promise::sf_awaited) { throw dpp::logic_exception("awaitable is already being awaited"); } @@ -682,7 +682,7 @@ template T awaitable::awaiter::await_resume() { auto &promise = *std::exchange(awaitable_obj.state_ptr, nullptr); - promise.state.fetch_and(~detail::promise::sf_awaited); + promise.state.fetch_and(~detail::promise::sf_awaited, std::memory_order::acq_rel); if (std::holds_alternative(promise.value)) { std::rethrow_exception(std::get<2>(promise.value)); } From fb59bbaa6cdbf428e6338780c39390fa31f5986d Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 6 Jul 2024 02:20:44 -0400 Subject: [PATCH 04/17] fix(coro): fix read-after-free in dpp::async --- include/dpp/coro/async.h | 10 ++++++++++ include/dpp/coro/awaitable.h | 7 +++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/include/dpp/coro/async.h b/include/dpp/coro/async.h index c5f8c283d9..92a3cfbe43 100644 --- a/include/dpp/coro/async.h +++ b/include/dpp/coro/async.h @@ -134,6 +134,16 @@ class async : public awaitable { explicit async(Fun &&fun, Args&&... args) : async{std::make_shared>()} { std::invoke(std::forward(fun), std::forward(args)..., api_callback); } + + async(const async&) = delete; + async(async&&) = default; + + async& operator=(const async&) = delete; + async& operator=(async&&) = default; + + ~async() { + this->abandon(); + } }; DPP_CHECK_ABI_COMPAT(async<>, async_dummy); diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 1859e996fd..1081a6cdd2 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -638,8 +638,11 @@ using promise = moveable_promise; template auto awaitable::abandon() -> uint8_t { - auto previous_state = state_ptr->state.fetch_or(state_flags::sf_broken, std::memory_order::acq_rel); - state_ptr = nullptr; + uint8_t previous_state = state_flags::sf_broken; + if (state_ptr) { + previous_state = state_ptr->state.fetch_or(state_flags::sf_broken, std::memory_order::acq_rel); + state_ptr = nullptr; + } return previous_state; } From 48d1dec841c2fb40450580679fac0dbc2be5d185 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 6 Jul 2024 02:35:11 -0400 Subject: [PATCH 05/17] improv(coro): add [[nodiscard]] to dpp::task, dpp::coroutine --- include/dpp/coro/coroutine.h | 2 +- include/dpp/coro/task.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/dpp/coro/coroutine.h b/include/dpp/coro/coroutine.h index dfe664e14a..0585544a47 100644 --- a/include/dpp/coro/coroutine.h +++ b/include/dpp/coro/coroutine.h @@ -70,7 +70,7 @@ using handle_t = std_coroutine::coroutine_handle>; * @tparam R Return type of the coroutine. Can be void, or a complete object that supports move construction and move assignment. */ template -class coroutine : public basic_awaitable> { +class [[nodiscard("dpp::coroutine only starts when it is awaited, it will do nothing if discarded")]] coroutine : public basic_awaitable> { /** * @brief Promise has friend access for the constructor */ diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index a7c251224a..f9a15ff9bb 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -92,7 +92,7 @@ template #ifndef _DOXYGEN_ requires (!std::is_reference_v) #endif -class task : public awaitable { +class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it, or its sync_wait method")]] task : public awaitable { friend struct detail::task::promise_t; using handle_t = detail::task::handle_t; From e4dc9f359236928ce249d7dc8cc504bd0d962fa5 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 6 Jul 2024 02:38:01 -0400 Subject: [PATCH 06/17] misc(coro): remove long-deprecated dpp::job event handlers --- include/dpp/event_router.h | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/include/dpp/event_router.h b/include/dpp/event_router.h index 1657fbd592..2babc42e29 100644 --- a/include/dpp/event_router.h +++ b/include/dpp/event_router.h @@ -478,7 +478,7 @@ template class event_router_t { * saving it in a variable is recommended to avoid variable lifetime issues. * * @details Example: @code{cpp} - * dpp::job my_handler(dpp::slashcommand_t event) { + * dpp::task<> my_handler(const dpp::slashcommand_t& event) { * co_await event.co_reply(dpp::message().add_component(dpp::component().add_component().set_label("click me!").set_id("test"))); * * dpp::button_click_t b = co_await c->on_button_click.with([](const dpp::button_click_t &event){ return event.custom_id == "test"; }); @@ -514,7 +514,7 @@ template class event_router_t { * * Example: * @details Example: @code{cpp} - * dpp::job my_handler(dpp::slashcommand_t event) { + * dpp::task<> my_handler(const dpp::slashcommand_t& event) { * co_await event.co_reply(dpp::message().add_component(dpp::component().add_component().set_label("click me!").set_id("test"))); * * dpp::button_click_t b = co_await c->on_message_create; @@ -607,7 +607,7 @@ template class event_router_t { * detach the listener from the event later if necessary. */ template - requires (utility::callable_returns || utility::callable_returns, const T&> || utility::callable_returns) + requires (utility::callable_returns, const T&> || utility::callable_returns) [[maybe_unused]] event_handle operator()(F&& fun) { return this->attach(std::forward(fun)); } @@ -649,28 +649,6 @@ template class event_router_t { dispatch_container.emplace(std::piecewise_construct, std::forward_as_tuple(h), std::forward_as_tuple(std::in_place_type_t{}, std::forward(fun))); return h; } - - /** - * @brief Attach a callable to the event, adding a listener. - * The callable should either be of the form `void(const T&)` or - * `dpp::task(const T&)`, where T is the event type for this event router. - * - * @deprecated dpp::job event handlers are deprecated and will be removed in a future version, use dpp::task instead. - * @param fun Callable to attach to event - * @return event_handle An event handle unique to this event, used to - * detach the listener from the event later if necessary. - */ - template - requires (utility::callable_returns) - DPP_DEPRECATED("dpp::job event handlers are deprecated and will be removed in a future version, use dpp::task instead") - [[maybe_unused]] event_handle attach(F&& fun) { - assert(dpp::utility::is_coro_enabled()); - - std::unique_lock l(mutex); - event_handle h = next_handle++; - dispatch_container.emplace(std::piecewise_construct, std::forward_as_tuple(h), std::forward_as_tuple(std::in_place_type_t{}, std::forward(fun))); - return h; - } # else /** * @brief Attach a callable to the event, adding a listener. From 6ab08aa227509c21d3ebdab6aadd1ca748a9843a Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Thu, 11 Jul 2024 10:11:08 -0400 Subject: [PATCH 07/17] improv(coro): add concept `awaitable_type`, move `dpp::detail::promise::promise` to `dpp::basic_promise`, document some more --- include/dpp/coro/awaitable.h | 101 ++++++++++++++++++++++++----------- include/dpp/coro/coro.h | 22 ++++++-- 2 files changed, 90 insertions(+), 33 deletions(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 1081a6cdd2..9ae78353ee 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -37,12 +37,12 @@ struct awaitable_dummy { #include +// Do not include as coro.h includes or depending on clang version #include #include #include -#include +#include #include -#include namespace dpp { @@ -91,20 +91,33 @@ class promise_base; */ struct empty{}; +/** + * @brief Variant for the 3 conceptual values of a coroutine: + */ +template +using result_t = std::variant, empty, T>, std::exception_ptr>; + template void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& result); } /* namespace detail::promise */ -template -requires (requires (Derived t) { detail::co_await_resolve(t); }) +template class basic_awaitable { protected: + /** + * @brief Implementation for sync_wait. This is code used by sync_wait, sync_wait_for, sync_wait_until. + * + * @tparam Timed Whether the wait function times out or not + * @param do_wait Function to do the actual wait on the cv + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. + */ template auto sync_wait_impl(auto&& do_wait) { using result_type = decltype(detail::co_await_resolve(std::declval()).await_resume()); using storage_type = std::conditional_t, detail::promise::empty, result_type>; - using variant_type = std::variant; + using variant_type = detail::promise::result_t; variant_type result; std::condition_variable cv; @@ -131,6 +144,8 @@ class basic_awaitable { * @brief Blocks this thread and waits for the awaitable to finish. * * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. */ auto sync_wait() { return sync_wait_impl([](std::condition_variable &cv, auto&& result) { @@ -145,8 +160,8 @@ class basic_awaitable { * * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. * @param duration Maximum duration to wait for - * @retval If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. - * @retval If T is non-void, returns a std::optional for which an absense of value means timed out. + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. */ template auto sync_wait_for(const std::chrono::duration& duration) { @@ -162,8 +177,8 @@ class basic_awaitable { * * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. * @param time Maximum time point to wait for - * @retval If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. - * @retval If T is non-void, returns a std::optional for which an absense of value means timed out. + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. */ template auto sync_wait_until(const std::chrono::time_point &time) { @@ -339,6 +354,9 @@ class awaitable : public basic_awaitable> { namespace detail::promise { +/** + * @brief Base class defining logic common to all promise types, aka the "write" end of an awaitable. + */ template class promise_base { protected: @@ -377,6 +395,12 @@ class promise_base { } } + /** + * @brief Unlinks this promise from its currently linked awaiter and returns it. + * + * At the time of writing this is only used in the case of a serious internal error in dpp::task. + * Avoid using this as this will crash if the promise is used after this. + */ std_coroutine::coroutine_handle<> release_awaiter() { return std::exchange(awaiter, nullptr); } @@ -393,6 +417,8 @@ class promise_base { /** * @brief Move construction is disabled. + * + * awaitable hold a pointer to this object so moving is not possible. */ promise_base(promise_base&& rhs) = delete; @@ -412,6 +438,7 @@ class promise_base { * * @tparam Notify Whether to resume any awaiter or not. * @throws dpp::logic_exception if the promise is not empty. + * @throws ? Any exception thrown by the coroutine if resumed will propagate */ template void set_exception(std::exception_ptr ptr) { @@ -427,9 +454,12 @@ class promise_base { /** * @brief Notify a currently awaiting coroutine that the result is ready. + * + * @note This may resume the coroutine on the current thread. + * @throws ? Any exception thrown by the coroutine if resumed will propagate */ void notify_awaiter() { - if (state.load(std::memory_order::acquire) & sf_awaited) { + if ((state.load(std::memory_order::acquire) & sf_awaited) != 0) { awaiter.resume(); } } @@ -449,6 +479,8 @@ class promise_base { } }; +} + /** * @brief Generic promise class, represents the owning potion of an asynchronous value. * @@ -459,10 +491,10 @@ class promise_base { * @see awaitable */ template -class promise : public promise_base { +class basic_promise : public detail::promise::promise_base { public: - using promise_base::promise_base; - using promise_base::operator=; + using detail::promise::promise_base::promise_base; + using detail::promise::promise_base::operator=; /** * @brief Construct the result in place by forwarding the arguments, and by default resume any awaiter. @@ -479,9 +511,9 @@ class promise : public promise_base { } catch (...) { this->value.template emplace<2>(std::current_exception()); } - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); + [[maybe_unused]] auto previous_value = this->state.fetch_or(detail::promise::sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { - if (previous_value & sf_awaited) { + if (previous_value & detail::promise::sf_awaited) { this->awaiter.resume(); } } @@ -510,11 +542,20 @@ class promise : public promise_base { } }; + +/** + * @brief Generic promise class, represents the owning potion of an asynchronous value. + * + * This class is roughly equivalent to std::promise, with the crucial distinction that the promise *IS* the shared state. + * As such, the promise needs to be kept alive for the entire time a value can be retrieved. + * + * @see awaitable + */ template <> -class promise : public promise_base { +class basic_promise : public detail::promise::promise_base { public: - using promise_base::promise_base; - using promise_base::operator=; + using detail::promise::promise_base::promise_base; + using detail::promise::promise_base::operator=; /** * @brief Set the promise to completed, and resume any awaiter. @@ -525,27 +566,30 @@ class promise : public promise_base { void set_value() { throw_if_not_empty(); this->value.emplace<1>(); - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); + [[maybe_unused]] auto previous_value = this->state.fetch_or(detail::promise::sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { - if (previous_value & sf_awaited) { + if (previous_value & detail::promise::sf_awaited) { this->awaiter.resume(); } } } }; -} - -template -using basic_promise = detail::promise::promise; - /** - * @brief Base class for a promise type. + * @brief Generic promise class, represents the owning potion of an asynchronous value. + * + * This class is roughly equivalent to std::promise, with the crucial distinction that the promise *IS* the shared state. + * As such, the promise needs to be kept alive for the entire time a value can be retrieved. + * + * The difference between basic_promise and this object is that this one is moveable as it wraps an underlying basic_promise in a std::unique_ptr. * - * Contains the base logic for @ref promise, but does not contain the set_value methods. + * @see awaitable */ template class moveable_promise { + /** + * @brief Shared state, wrapped in a unique_ptr to allow move without disturbing an awaitable's promise pointer. + */ std::unique_ptr> shared_state = std::make_unique>(); public: @@ -712,9 +756,6 @@ namespace dpp { namespace detail::promise { -template -using result_t = std::variant, empty, T>, std::exception_ptr>; - template void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& result) { [](auto* awaitable_, std::condition_variable &cv_, auto&& result_) -> dpp::job { diff --git a/include/dpp/coro/coro.h b/include/dpp/coro/coro.h index 76f3256ed8..cf5ab2cb51 100644 --- a/include/dpp/coro/coro.h +++ b/include/dpp/coro/coro.h @@ -114,13 +114,14 @@ decltype(auto) co_await_resolve(T&& expr) noexcept { } #else + /** * @brief Concept to check if a type has a useable `operator co_await()` member * * @note This is actually a C++20 concept but Doxygen doesn't do well with them */ template -bool has_co_await_member; +inline constexpr bool has_co_await_member; /** * @brief Concept to check if a type has a useable overload of the free function `operator co_await(expr)` @@ -128,7 +129,7 @@ bool has_co_await_member; * @note This is actually a C++20 concept but Doxygen doesn't do well with them */ template -bool has_free_co_await; +inline constexpr bool has_free_co_await; /** * @brief Concept to check if a type has useable `await_ready()`, `await_suspend()` and `await_resume()` member functions. @@ -136,7 +137,15 @@ bool has_free_co_await; * @note This is actually a C++20 concept but Doxygen doesn't do well with them */ template -bool has_await_members; +inline constexpr bool has_await_members; + +/** + * @brief Concept to check if a type can be used with co_await + * + * @note This is actually a C++20 concept but Doxygen doesn't do well with them + */ +template +inline constexpr bool awaitable_type; /** * @brief Mimics the compiler's behavior of using co_await. That is, it returns whichever works first, in order : `expr.operator co_await();` > `operator co_await(expr)` > `expr` @@ -144,6 +153,7 @@ bool has_await_members; * This function is conditionally noexcept, if the returned expression also is. */ decltype(auto) co_await_resolve(auto&& expr) {} + #endif /** @@ -154,6 +164,12 @@ using awaitable_result = decltype(co_await_resolve(std::declval()).await_resu } // namespace detail +/** + * @brief Concept to check if a type can be used with co_await + */ +template +concept awaitable_type = requires (T expr) { detail::co_await_resolve(expr); }; + struct confirmation_callback_t; template From b56b6bf414780ffb8204789368a998c706077184 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Thu, 11 Jul 2024 11:12:28 -0400 Subject: [PATCH 08/17] improv(coro): allow move-only types in dpp::async, de-duplicate void specializations where possible, document some more --- include/dpp/coro/async.h | 54 ++++++++++++++++++------ include/dpp/coro/awaitable.h | 81 ++++++++++-------------------------- include/dpp/coro/coro.h | 33 ++++++++++++++- include/dpp/coro/coroutine.h | 11 ++++- include/dpp/coro/task.h | 2 +- 5 files changed, 108 insertions(+), 73 deletions(-) diff --git a/include/dpp/coro/async.h b/include/dpp/coro/async.h index 92a3cfbe43..a4444a6f65 100644 --- a/include/dpp/coro/async.h +++ b/include/dpp/coro/async.h @@ -53,22 +53,30 @@ namespace async { */ template struct callback { + /** + * @brief Promise object to set the result into + */ std::shared_ptr> promise{nullptr}; - void operator()(const R& v) const { + /** + * @brief Call operator, sets the value in the promise and notifies any awaiter + */ + void operator()(const detail::argument& v) const requires (detail::is_copy_constructible) { promise->set_value(v); } - - void operator()(R&& v) const { + + /** + * @brief Call operator, sets the value in the promise and notifies any awaiter + */ + void operator()(detail::argument&& v) const requires (detail::is_move_constructible) { promise->set_value(std::move(v)); } -}; - -template <> -struct callback { - std::shared_ptr> promise{nullptr}; - - void operator()() const { + + /** + * @brief Call operator, sets the value in the promise and notifies any awaiter + */ + void operator()() const requires (std::is_void_v) + { promise->set_value(); } }; @@ -91,9 +99,14 @@ struct confirmation_callback_t; */ template class async : public awaitable { - + /** + * @brief Callable object to pass to API calls + */ detail::async::callback api_callback{}; + /** + * @brief Internal promise constructor, grabs a promise object for the callback to use + */ explicit async(std::shared_ptr> &&promise) : awaitable{promise.get()}, api_callback{std::move(promise)} {} public: @@ -135,12 +148,29 @@ class async : public awaitable { std::invoke(std::forward(fun), std::forward(args)..., api_callback); } + /** + * @brief Copy constructor is disabled. + */ async(const async&) = delete; - async(async&&) = default; + /** + * @brief Move constructor, moves the awaitable async object + */ + async(async&&) = default; + + /** + * @brief Copy assignment operator is disabled. + */ async& operator=(const async&) = delete; + + /** + * @brief Move assignment operator, moves the awaitable async object + */ async& operator=(async&&) = default; + /** + * @brief Destructor, signals to the callback that the async object is gone and shouldn't be notified of the result + */ ~async() { this->abandon(); } diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 9ae78353ee..16fc58b70a 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -123,6 +123,9 @@ class basic_awaitable { detail::promise::spawn_sync_wait_job(static_cast(this), cv, result); do_wait(cv, result); + /* + * Note: we use .index() here to support dpp::promise & dpp::promise :D + */ if (result.index() == 2) { std::rethrow_exception(std::get<2>(result)); } @@ -365,12 +368,14 @@ class promise_base { /** * @brief Variant representing one of either 3 states of the result value : empty, result, exception. */ - using storage_type = std::variant, empty, T>, std::exception_ptr>; + using storage_type = result_t; /** * @brief State of the result value. * * @see storage_type + * + * @note use .index() instead of std::holds_alternative to support promise_base and promise_base :) */ storage_type value = std::monostate{}; @@ -526,7 +531,8 @@ class basic_promise : public detail::promise::promise_base { * @throws dpp::logic_exception if the promise is not empty. */ template - void set_value(const T& v) requires (std::copy_constructible) { + requires (detail::is_copy_constructible) + void set_value(const detail::argument& v) { emplace_value(v); } @@ -537,35 +543,22 @@ class basic_promise : public detail::promise::promise_base { * @throws dpp::logic_exception if the promise is not empty. */ template - void set_value(T&& v) requires (std::move_constructible) { + requires (detail::is_move_constructible) + void set_value(detail::argument&& v) { emplace_value(std::move(v)); } -}; - - -/** - * @brief Generic promise class, represents the owning potion of an asynchronous value. - * - * This class is roughly equivalent to std::promise, with the crucial distinction that the promise *IS* the shared state. - * As such, the promise needs to be kept alive for the entire time a value can be retrieved. - * - * @see awaitable - */ -template <> -class basic_promise : public detail::promise::promise_base { -public: - using detail::promise::promise_base::promise_base; - using detail::promise::promise_base::operator=; /** - * @brief Set the promise to completed, and resume any awaiter. + * @brief Construct the result by move, and resume any awaiter. * + * @tparam Notify Whether to resume any awaiter or not. * @throws dpp::logic_exception if the promise is not empty. */ template + requires (std::is_void_v) void set_value() { - throw_if_not_empty(); - this->value.emplace<1>(); + this->throw_if_not_empty(); + this->value.template emplace<1>(); [[maybe_unused]] auto previous_value = this->state.fetch_or(detail::promise::sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { if (previous_value & detail::promise::sf_awaited) { @@ -606,7 +599,7 @@ class moveable_promise { * @copydoc basic_promise::set_value(const T&) */ template - void set_value(const T& v) requires (std::copy_constructible) { + void set_value(const detail::argument& v) requires (detail::is_copy_constructible) { shared_state->template set_value(v); } @@ -614,52 +607,24 @@ class moveable_promise { * @copydoc basic_promise::set_value(T&&) */ template - void set_value(T&& v) requires (std::move_constructible) { + void set_value(detail::argument&& v) requires (detail::is_move_constructible) { shared_state->template set_value(std::move(v)); } /** - * @copydoc basic_promise::set_value(T&&) + * @copydoc basic_promise::set_value() */ template - void set_exception(std::exception_ptr ptr) { - shared_state->template set_exception(std::move(ptr)); + void set_value() requires (std::is_void_v) { + shared_state->template set_value(); } /** - * @copydoc basic_promise::notify_awaiter - */ - void notify_awaiter() { - shared_state->notify_awaiter(); - } - - /** - * @copydoc basic_promise::get_awaitable - */ - awaitable get_awaitable() { - return shared_state->get_awaitable(); - } -}; - -template <> -class moveable_promise { - std::unique_ptr> shared_state = std::make_unique>(); - -public: - /** - * @copydoc basic_promise::set_value - */ - template - void set_value() { - shared_state->set_value(); - } - - /** - * @copydoc basic_promise::set_exception + * @copydoc basic_promise::set_value(T&&) */ template void set_exception(std::exception_ptr ptr) { - shared_state->set_exception(std::move(ptr)); + shared_state->template set_exception(std::move(ptr)); } /** @@ -672,7 +637,7 @@ class moveable_promise { /** * @copydoc basic_promise::get_awaitable */ - awaitable get_awaitable() { + awaitable get_awaitable() { return shared_state->get_awaitable(); } }; diff --git a/include/dpp/coro/coro.h b/include/dpp/coro/coro.h index cf5ab2cb51..5d9fefa9a6 100644 --- a/include/dpp/coro/coro.h +++ b/include/dpp/coro/coro.h @@ -19,8 +19,8 @@ * ************************************************************************************/ -#ifdef DPP_CORO #pragma once +#ifdef DPP_CORO #if (defined(_LIBCPP_VERSION) and !defined(__cpp_impl_coroutine)) // if libc++ experimental implementation (LLVM < 14) # define STDCORO_EXPERIMENTAL_HEADER @@ -113,6 +113,37 @@ decltype(auto) co_await_resolve(T&& expr) noexcept { return static_cast(expr); } +/** + * @brief Helper for when we need to pass an argument that may be void at parsing + * e.g. `void set_value(const detail::argument& v) requires (std::copy_constructible)`) would break without this + */ +template +using argument = std::conditional_t, std::monostate, std::type_identity_t>; + +/** + * @brief Helper because clang15 chokes on std::copy_constructible + */ +template +inline constexpr bool is_copy_constructible = std::copy_constructible; + +/** + * @brief Helper because clang15 chokes on std::copy_constructible + */ +template <> +inline constexpr bool is_copy_constructible = false; + +/** + * @brief Helper because clang15 chokes on std::move_constructible + */ +template +inline constexpr bool is_move_constructible = std::move_constructible; + +/** + * @brief Helper because clang15 chokes on std::move_constructible + */ +template <> +inline constexpr bool is_move_constructible = false; + #else /** diff --git a/include/dpp/coro/coroutine.h b/include/dpp/coro/coroutine.h index 0585544a47..a9e7a5d100 100644 --- a/include/dpp/coro/coroutine.h +++ b/include/dpp/coro/coroutine.h @@ -47,7 +47,7 @@ namespace detail { namespace coroutine { - template +template struct promise_t; template @@ -87,6 +87,9 @@ class [[nodiscard("dpp::coroutine only starts when it is awaited, it will do not coroutine(detail::coroutine::handle_t h) : handle{h} {} struct awaiter { + /** + * @brief Reference to the coroutine object being awaited. + */ coroutine &coro; /** @@ -117,6 +120,12 @@ class [[nodiscard("dpp::coroutine only starts when it is awaited, it will do not return coro.handle; } + /** + * @brief Final function called by the standard library when the coroutine is co_await-ed. + * + * Pops the coroutine's result and returns it. + * @remark Do not call this manually, use the co_await keyword instead. + */ R await_resume() { detail::coroutine::promise_t &promise = coro.handle.promise(); if (promise.exception) { diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index f9a15ff9bb..1a1412be6b 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -375,7 +375,7 @@ struct promise_t : promise_base { /** * @brief Function called by the standard library when the coroutine co_returns * - * Does nothing but is required by the standard library. + * Sets the promise state to finished. */ void return_void() noexcept { set_value(); From 817c00c06ccc42c55470805f216b924c9499fe43 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Thu, 11 Jul 2024 11:46:40 -0400 Subject: [PATCH 09/17] improv(coro): thanks https://www.includecpp.org/! --- include/dpp/coro/async.h | 10 ++++++++-- include/dpp/coro/awaitable.h | 20 ++++++++++---------- include/dpp/coro/coro.h | 31 ------------------------------- 3 files changed, 18 insertions(+), 43 deletions(-) diff --git a/include/dpp/coro/async.h b/include/dpp/coro/async.h index a4444a6f65..4295ae1218 100644 --- a/include/dpp/coro/async.h +++ b/include/dpp/coro/async.h @@ -60,15 +60,21 @@ struct callback { /** * @brief Call operator, sets the value in the promise and notifies any awaiter + * + * @param v Callback value */ - void operator()(const detail::argument& v) const requires (detail::is_copy_constructible) { + template + void operator()(const U& v) const requires (std::convertible_to) { promise->set_value(v); } /** * @brief Call operator, sets the value in the promise and notifies any awaiter + * + * @param v Callback value */ - void operator()(detail::argument&& v) const requires (detail::is_move_constructible) { + template + void operator()(U&& v) const requires (std::convertible_to) { promise->set_value(std::move(v)); } diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 16fc58b70a..d4b1f22750 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -530,9 +530,9 @@ class basic_promise : public detail::promise::promise_base { * @tparam Notify Whether to resume any awaiter or not. * @throws dpp::logic_exception if the promise is not empty. */ - template - requires (detail::is_copy_constructible) - void set_value(const detail::argument& v) { + template + requires (std::convertible_to) + void set_value(const U& v) { emplace_value(v); } @@ -542,9 +542,9 @@ class basic_promise : public detail::promise::promise_base { * @tparam Notify Whether to resume any awaiter or not. * @throws dpp::logic_exception if the promise is not empty. */ - template - requires (detail::is_move_constructible) - void set_value(detail::argument&& v) { + template + requires (std::convertible_to) + void set_value(U&& v) { emplace_value(std::move(v)); } @@ -598,16 +598,16 @@ class moveable_promise { /** * @copydoc basic_promise::set_value(const T&) */ - template - void set_value(const detail::argument& v) requires (detail::is_copy_constructible) { + template + void set_value(const U& v) requires (std::convertible_to) { shared_state->template set_value(v); } /** * @copydoc basic_promise::set_value(T&&) */ - template - void set_value(detail::argument&& v) requires (detail::is_move_constructible) { + template + void set_value(U&& v) requires (std::convertible_to) { shared_state->template set_value(std::move(v)); } diff --git a/include/dpp/coro/coro.h b/include/dpp/coro/coro.h index 5d9fefa9a6..cf5a9beaaf 100644 --- a/include/dpp/coro/coro.h +++ b/include/dpp/coro/coro.h @@ -113,37 +113,6 @@ decltype(auto) co_await_resolve(T&& expr) noexcept { return static_cast(expr); } -/** - * @brief Helper for when we need to pass an argument that may be void at parsing - * e.g. `void set_value(const detail::argument& v) requires (std::copy_constructible)`) would break without this - */ -template -using argument = std::conditional_t, std::monostate, std::type_identity_t>; - -/** - * @brief Helper because clang15 chokes on std::copy_constructible - */ -template -inline constexpr bool is_copy_constructible = std::copy_constructible; - -/** - * @brief Helper because clang15 chokes on std::copy_constructible - */ -template <> -inline constexpr bool is_copy_constructible = false; - -/** - * @brief Helper because clang15 chokes on std::move_constructible - */ -template -inline constexpr bool is_move_constructible = std::move_constructible; - -/** - * @brief Helper because clang15 chokes on std::move_constructible - */ -template <> -inline constexpr bool is_move_constructible = false; - #else /** From 62ecaf002d2f80bd64cba3f8cebf4a21e85c13e6 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Thu, 11 Jul 2024 11:50:57 -0400 Subject: [PATCH 10/17] fix(coro): fix unused alias warning --- include/dpp/coro/awaitable.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index d4b1f22750..baf573e9e5 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -116,7 +116,6 @@ class basic_awaitable { template auto sync_wait_impl(auto&& do_wait) { using result_type = decltype(detail::co_await_resolve(std::declval()).await_resume()); - using storage_type = std::conditional_t, detail::promise::empty, result_type>; using variant_type = detail::promise::result_t; variant_type result; std::condition_variable cv; From 1afb0c28b98151c5e0db5209a2298a98a8f1745d Mon Sep 17 00:00:00 2001 From: Craig Edwards Date: Fri, 12 Jul 2024 10:39:21 +0000 Subject: [PATCH 11/17] coro test case to detect leaks --- src/coro_loop/coro_loop.cpp | 73 +++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 src/coro_loop/coro_loop.cpp diff --git a/src/coro_loop/coro_loop.cpp b/src/coro_loop/coro_loop.cpp new file mode 100644 index 0000000000..159c801cf3 --- /dev/null +++ b/src/coro_loop/coro_loop.cpp @@ -0,0 +1,73 @@ +/************************************************************************************ + * + * D++, A Lightweight C++ library for Discord + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2021 Craig Edwards and D++ contributors + * (https://github.com/brainboxdotcc/DPP/graphs/contributors) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ************************************************************************************/ + +#include +#include + +int64_t proc_self_value(const std::string& find_token) { + int64_t ret = 0; + std::ifstream self_status("/proc/self/status"); + while (self_status) { + std::string token; + self_status >> token; + if (token == find_token) { + self_status >> ret; + break; + } + } + self_status.close(); + return ret; +} + +int64_t rss() { + return proc_self_value("VmRSS:") * 1024; +} + +#ifdef DPP_CORO +dpp::task test(dpp::cluster& cluster) { + [[maybe_unused]] int test[102400]{}; + test[60] = 5; + co_return; +} +#endif + +int main() { +#ifdef DPP_CORO + char* t = getenv("DPP_UNIT_TEST_TOKEN"); + if (t) { + dpp::cluster coro_cluster(t, dpp::i_guilds, 1, 0, 1, true, dpp::cache_policy::cpol_none); + coro_cluster.set_websocket_protocol(dpp::ws_etf); + coro_cluster.on_log(dpp::utility::cout_logger()); + coro_cluster.on_ready([&coro_cluster](auto) -> dpp::task { + coro_cluster.start_timer([&coro_cluster](dpp::timer) -> dpp::task { + for (int i = 0; i < 1000; ++i) { + co_await test(coro_cluster); + } + coro_cluster.log(dpp::ll_info, "coro timer ticked. RSS=" + std::to_string(rss())); + co_return; + }, 1); + co_return; + }); + coro_cluster.start(dpp::st_wait); + } +#endif +} From af38de011db8df2b5311a572e73a6240ec3ff4ff Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 13 Jul 2024 11:21:28 -0400 Subject: [PATCH 12/17] improv: add requires clause to await_transform to improve error messages when trying to co_await something that can't be --- include/dpp/coro/awaitable.h | 2 +- include/dpp/coro/coro.h | 2 +- include/dpp/coro/task.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index baf573e9e5..20b091decd 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -102,7 +102,7 @@ void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& re } /* namespace detail::promise */ -template +template class basic_awaitable { protected: /** diff --git a/include/dpp/coro/coro.h b/include/dpp/coro/coro.h index cf5a9beaaf..34827debdd 100644 --- a/include/dpp/coro/coro.h +++ b/include/dpp/coro/coro.h @@ -168,7 +168,7 @@ using awaitable_result = decltype(co_await_resolve(std::declval()).await_resu * @brief Concept to check if a type can be used with co_await */ template -concept awaitable_type = requires (T expr) { detail::co_await_resolve(expr); }; +concept awaitable_type = requires (T expr) { detail::co_await_resolve(expr).await_ready(); }; struct confirmation_callback_t; diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index 1a1412be6b..9225f2d053 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -297,7 +297,7 @@ struct promise_base : basic_promise { * * @return @ref proxy_awaiter Returns a proxy awaiter that will check for cancellation on resumption */ - template + template [[nodiscard]] auto await_transform(T&& expr) const noexcept(noexcept(co_await_resolve(std::forward(expr)))) { using awaitable_t = decltype(co_await_resolve(std::forward(expr))); return proxy_awaiter{*this, co_await_resolve(std::forward(expr))}; From d5767113a3dd5e385ddb3627b4d1dab4cc998cb1 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 13 Jul 2024 12:11:40 -0400 Subject: [PATCH 13/17] fix(coro): fix leak of the coroutine frame, add if_this_causes_an_invalid_read_your_promise_was_destroyed_before_your_awaitable____check_your_promise_lifetime --- include/dpp/coro/awaitable.h | 15 +++++++++++---- include/dpp/coro/task.h | 2 +- src/unittest/coro.cpp | 13 +++++++++---- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 20b091decd..4ba8ed79a9 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -294,6 +294,15 @@ class awaitable : public basic_awaitable> { awaitable(awaitable&& rhs) noexcept : state_ptr(std::exchange(rhs.state_ptr, nullptr)) { } + /** + * @brief Title :) + * + * We use this in the destructor + */ + void if_this_causes_an_invalid_read_your_promise_was_destroyed_before_your_awaitable____check_your_promise_lifetime() { + abandon(); + } + /** * @brief Destructor. * @@ -656,9 +665,7 @@ auto awaitable::abandon() -> uint8_t { template awaitable::~awaitable() { - if (state_ptr) { - state_ptr->state.fetch_or(state_flags::sf_broken, std::memory_order::acq_rel); - } + if_this_causes_an_invalid_read_your_promise_was_destroyed_before_your_awaitable____check_your_promise_lifetime(); } template @@ -691,7 +698,7 @@ bool awaitable::awaiter::await_suspend(detail::std_coroutine::corout template template T awaitable::awaiter::await_resume() { - auto &promise = *std::exchange(awaitable_obj.state_ptr, nullptr); + auto &promise = *awaitable_obj.state_ptr; promise.state.fetch_and(~detail::promise::sf_awaited, std::memory_order::acq_rel); if (std::holds_alternative(promise.value)) { diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index 9225f2d053..8e6c862b3d 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -247,7 +247,7 @@ struct promise_base : basic_promise { * Stores the exception pointer to rethrow on co_await. If the task object is destroyed and was not cancelled, throw instead */ void unhandled_exception() { - if ((this->state.load() == promise::state_flags::sf_broken) && !cancelled) { + if ((this->state.load() & promise::state_flags::sf_broken) && !cancelled) { throw; } this->template set_exception(std::current_exception()); diff --git a/src/unittest/coro.cpp b/src/unittest/coro.cpp index ea35e919ca..eead3c85b6 100644 --- a/src/unittest/coro.cpp +++ b/src/unittest/coro.cpp @@ -147,6 +147,11 @@ dpp::task task_offline_test() { if (int ret = co_await simple_awaitable{test, 42}; ret != 42) { set_status(test, ts_failed, "failed simple awaitable"); } + + if (int ret = co_await []() -> dpp::task { co_return 42; }(); ret != 42) { + set_status(test, ts_failed, "failed simple task"); + } + std::array, 10> tasks; auto start = clock::now(); for (int i = 0; i < 10; ++i) { @@ -408,9 +413,9 @@ dpp::job coro_awaitable_test() { { dpp::promise test; dpp::awaitable awaitable{test.get_awaitable()}; - std::thread th{[p = std::move(test)]() mutable { + std::thread th{[&test]() mutable { std::this_thread::sleep_for(std::chrono::seconds(2)); - p.set_value(69); + test.set_value(69); }}; th.detach(); if (std::optional res = awaitable.sync_wait_for(std::chrono::seconds(5)); !res || *res != 69) { @@ -450,9 +455,9 @@ dpp::job coro_awaitable_test() { { dpp::promise test; dpp::awaitable awaitable{test.get_awaitable()}; - std::thread th{[p = std::move(test)]() mutable { + std::thread th{[&test]() mutable { std::this_thread::sleep_for(std::chrono::seconds(2)); - p.set_exception(std::make_exception_ptr(dpp::voice_exception("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))); + test.set_exception(std::make_exception_ptr(dpp::voice_exception("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))); }}; th.detach(); bool success = false; From f85755456a09484f7584cdfca54da4230e0c3d14 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Sat, 13 Jul 2024 16:20:50 -0400 Subject: [PATCH 14/17] fix(coro): fix dpp::task's move assignment --- include/dpp/coro/awaitable.h | 2 +- include/dpp/coro/task.h | 29 +++++++++++++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 4ba8ed79a9..f37bc93e66 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -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(); } } diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index 8e6c862b3d..324c4957ca 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -106,6 +106,20 @@ class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it, */ explicit task(handle_t handle_) : awaitable(&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. @@ -136,6 +150,7 @@ class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it, */ task &operator=(task &&other) noexcept { awaitable::operator=(std::move(other)); + cleanup(); handle = std::exchange(other.handle, nullptr); return *this; } @@ -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(); } /** @@ -406,14 +415,14 @@ std_coroutine::coroutine_handle<> final_awaiter::await_suspend(handle_t ha promise_t &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(); From 1899e6aabeda3cb5917c74a56a5ee0883e6358b1 Mon Sep 17 00:00:00 2001 From: Craig Edwards Date: Mon, 15 Jul 2024 16:07:27 +0000 Subject: [PATCH 15/17] change get_gateway_bot to use coro when available --- src/dpp/cluster.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/dpp/cluster.cpp b/src/dpp/cluster.cpp index 83d53a0528..4866e7a67b 100644 --- a/src/dpp/cluster.cpp +++ b/src/dpp/cluster.cpp @@ -190,7 +190,12 @@ void cluster::start(bool return_after) { /* Start up all shards */ gateway g; try { +#ifdef DPP_CORO + confirmation_callback_t cc = co_get_gateway_bot().sync_wait(); + g = std::get(cc.value); +#else g = dpp::sync(this, &cluster::get_gateway_bot); +#endif log(ll_debug, "Cluster: " + std::to_string(g.session_start_remaining) + " of " + std::to_string(g.session_start_total) + " session starts remaining"); if (g.session_start_remaining < g.shards) { throw dpp::connection_exception(err_no_sessions_left, "Discord indicates you cannot start enough sessions to boot this cluster! Cluster startup aborted. Try again later."); From 6f069905c3ea12c9fb0a965382d3ca7dd3e20ba5 Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Mon, 15 Jul 2024 12:54:55 -0400 Subject: [PATCH 16/17] fix(coro): fix awaitable/task move assignment, again --- include/dpp/coro/awaitable.h | 1 + include/dpp/coro/task.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index f37bc93e66..2fefb1fd4e 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -322,6 +322,7 @@ class awaitable : public basic_awaitable> { * @return *this */ awaitable& operator=(awaitable&& rhs) noexcept { + abandon(); state_ptr = std::exchange(rhs.state_ptr, nullptr); return *this; } diff --git a/include/dpp/coro/task.h b/include/dpp/coro/task.h index 324c4957ca..6625bb1f3e 100644 --- a/include/dpp/coro/task.h +++ b/include/dpp/coro/task.h @@ -149,9 +149,9 @@ class [[nodiscard("dpp::task cancels itself on destruction. use co_await on it, * @param other Task to move the handle from */ task &operator=(task &&other) noexcept { - awaitable::operator=(std::move(other)); cleanup(); handle = std::exchange(other.handle, nullptr); + awaitable::operator=(std::move(other)); return *this; } From 2d01764d96e533593094059a6baec01ff78bfeb6 Mon Sep 17 00:00:00 2001 From: Miuna <809711+Mishura4@users.noreply.github.com> Date: Thu, 18 Jul 2024 13:42:44 -0400 Subject: [PATCH 17/17] fix: add and to misc-enum.h --- include/dpp/misc-enum.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/dpp/misc-enum.h b/include/dpp/misc-enum.h index 8f4f160f5a..19fd2fcd9a 100644 --- a/include/dpp/misc-enum.h +++ b/include/dpp/misc-enum.h @@ -21,7 +21,8 @@ ************************************************************************************/ #pragma once #include -#include +#include +#include namespace dpp {