Skip to content

Commit

Permalink
refactor(coro): remove ability for dpp::task to detach
Browse files Browse the repository at this point in the history
  • Loading branch information
Mishura4 committed Aug 15, 2023
1 parent 58ef5e4 commit 28a859b
Show file tree
Hide file tree
Showing 9 changed files with 324 additions and 280 deletions.
2 changes: 1 addition & 1 deletion buildtools/classes/Generator/CoroGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public function checkForChanges(): bool
*/
public function generateHeaderDef(string $returnType, string $currentFunction, string $parameters, string $noDefaults, string $parameterTypes, string $parameterNames): string
{
return "awaitable<confirmation_callback_t> co_{$currentFunction}($parameters);\n\n";
return "[[nodiscard]] awaitable<confirmation_callback_t> co_{$currentFunction}($parameters);\n\n";
}

/**
Expand Down
376 changes: 188 additions & 188 deletions include/dpp/cluster_coro_calls.h

Large diffs are not rendered by default.

14 changes: 9 additions & 5 deletions include/dpp/coro/async.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ struct confirmation_callback_t;
* This class is the return type of the dpp::cluster::co_* methods, but it can also be created manually to wrap any async call.
*
* @remark - This object's methods, other than constructors and operators, should not be called directly. It is designed to be used with coroutine keywords such as co_await.
* @remark - This object must not be co_await-ed more than once.
* @remark - The coroutine may be resumed in another thread, do not rely on thread_local variables.
* @warning This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @warning - Using co_await on this object more than once is undefined behavior.
* @tparam R The return type of the API call. Defaults to confirmation_callback_t
*/
template <typename R>
Expand All @@ -50,6 +50,8 @@ class async {
* @brief Ref-counted callback, contains the callback logic and manages the lifetime of the callback data over multiple threads.
*/
struct shared_callback {
struct empty_tag_t{};

/**
* @brief State of the async and its callback.
*/
Expand Down Expand Up @@ -115,6 +117,8 @@ class async {
*/
shared_callback() : state{new callback_state{.ref_count = 1}} {}

shared_callback(empty_tag_t) noexcept : state{nullptr} {}

/**
* @brief Destructor. Releases the held reference and destroys if no other references exist.
*/
Expand Down Expand Up @@ -198,7 +202,7 @@ class async {
/**
* @brief Shared state of the async and its callback, to be used across threads.
*/
shared_callback api_callback;
shared_callback api_callback{nullptr};

public:
/**
Expand Down Expand Up @@ -239,6 +243,8 @@ class async {
std::invoke(awaitable.request, api_callback);
}

async() noexcept : api_callback{typename shared_callback::empty_tag_t{}} {}

/**
* @brief Destructor. If any callback is pending it will be aborted.
*/
Expand Down Expand Up @@ -304,8 +310,6 @@ class async {

if (api_callback.get_result().has_value())
return false; // immediately resume the coroutine as we already have the result of the api call
if constexpr (requires (T t) { t.is_sync = false; })
caller.promise().is_sync = false;
api_callback.state->coro_handle = caller;
return true; // suspend the caller, the callback will resume it
}
Expand Down
4 changes: 1 addition & 3 deletions include/dpp/coro/awaitable.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct confirmation_callback_t;
* @brief A co_await-able object handling an API call.
*
* @remark - The coroutine may be resumed in another thread, do not rely on thread_local variables.
* @warning This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @tparam R The return type of the API call. Defaults to confirmation_callback_t
*/
template <typename R>
Expand Down Expand Up @@ -112,8 +112,6 @@ struct awaitable {
*/
template <typename T>
void await_suspend(detail::std_coroutine::coroutine_handle<T> caller) noexcept(noexcept(std::invoke(fun, std::declval<std::function<void(R)>&&>()))) {
if constexpr (requires (T promise) {{promise.is_sync} -> std::same_as<bool&>;})
caller.promise().is_sync = false;
std::invoke(fun, [this, caller](auto &&api_result) {
result = api_result;
caller.resume();
Expand Down
10 changes: 10 additions & 0 deletions include/dpp/coro/coro.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ class task;

struct job;

#ifdef DPP_CORO_TEST
/**
* @brief Allocation count of a certain type, for testing purposes.
*
* @todo Remove when coro is stable
*/
template <typename T>
inline int coro_alloc_count = 0;
#endif

} // namespace dpp

#endif /* DPP_CORO */
Expand Down
42 changes: 29 additions & 13 deletions include/dpp/coro/coroutine.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ namespace detail {
/**
* @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 <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @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 <typename R>
Expand Down Expand Up @@ -105,19 +106,22 @@ class coroutine {
*/
coroutine &operator=(coroutine &&other) noexcept {
handle = std::exchange(other.handle, nullptr);
return (*this);
return *this;
}

/**
* @brief First function called by the standard library when the task 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 true Always suspend, we need to start the coroutine.
*/
bool await_ready() const noexcept {
assert(handle && "cannot co_await an empty coroutine");
assert(!handle.done() && "cannot co_await a finished coroutine");
return (false);
bool await_ready() const {
if (!handle)
throw dpp::invalid_operation_exception("cannot co_await an empty coroutine");
if (handle.done())
throw dpp::invalid_operation_exception("cannot co_await a finished coroutine");
return false;
}

/**
Expand All @@ -129,11 +133,9 @@ class coroutine {
* @param caller The calling coroutine, now suspended
*/
template <typename T>
void await_suspend(detail::std_coroutine::coroutine_handle<T> caller) {
if constexpr (requires (T t) { t.is_sync = false; })
caller.promise().is_sync = false;
detail::coroutine_handle<R> await_suspend(detail::std_coroutine::coroutine_handle<T> caller) {
handle.promise().parent = caller;
handle.resume();
return handle;
}

/**
Expand All @@ -154,7 +156,11 @@ class coroutine {
namespace detail {
template <typename R>
struct coroutine_final_awaiter;


#ifdef DPP_CORO_TEST
struct coroutine_promise_base{};
#endif

/**
* @brief Promise type for coroutine.
*/
Expand All @@ -175,6 +181,16 @@ namespace detail {
*/
std::exception_ptr exception{nullptr};

#ifdef DPP_CORO_TEST
coroutine_promise() {
++coro_alloc_count<coroutine_promise_base>;
}

~coroutine_promise() {
--coro_alloc_count<coroutine_promise_base>;
}
#endif

/**
* @brief Function called by the standard library when reaching the end of a coroutine
*
Expand Down Expand Up @@ -228,7 +244,7 @@ namespace detail {
* @return false Always return false, we need to suspend
*/
bool await_ready() const noexcept {
return (false);
return false;
}

/**
Expand All @@ -243,7 +259,7 @@ namespace detail {
}

/**
* @brief Function (never) called by the standard library when the coroutine is (never) resumed
* @brief Function called by the standard library when this object is resumed
*/
void await_resume() const noexcept {}
};
Expand Down
26 changes: 25 additions & 1 deletion include/dpp/coro/job.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ namespace dpp {
*
* This object stores no state and is the recommended way to use coroutines if you do not need to co_await the result.
*
* @warning It cannot be co_awaited, which means the second it co_awaits something, the program jumps back to the calling function, which continues executing.
* @warning - This feature is EXPERIMENTAL. The API may change at any time and there may be bugs. Please report any to <a href="https://github.com/brainboxdotcc/DPP/issues">GitHub issues</a> or to the <a href="https://discord.gg/dpp">D++ Discord server</a>.
* @warning - It cannot be co_awaited, which means the second it co_awaits something, the program jumps back to the calling function, which continues executing.
* At this point, if the function returns, every object declared in the function including its parameters are destroyed, which causes dangling references.
* This is exactly the same problem as references in lambdas : https://dpp.dev/lambdas-and-locals.html.
* For this reason, `co_await` will error if any parameters are passed by reference.
Expand All @@ -45,11 +46,26 @@ struct job {};

namespace detail {

#ifdef DPP_CORO_TEST
struct job_promise_base{};
#endif

/**
* @brief Coroutine promise type for a job
*/
template <bool has_reference_params>
struct job_promise {

#ifdef DPP_CORO_TEST
job_promise() {
++coro_alloc_count<job_promise_base>;
}

~job_promise() {
--coro_alloc_count<job_promise_base>;
}
#endif

/*
* @brief Function called when the job is done.
*
Expand Down Expand Up @@ -112,6 +128,14 @@ struct job_promise {

} // namespace dpp

template <>
struct dpp::detail::std_coroutine::coroutine_traits<dpp::job> {
/**
* @brief Promise type for this coroutine signature.
*/
using promise_type = dpp::detail::job_promise<false>;
};

/**
* @brief Specialization of std::coroutine_traits, helps the standard library figure out a promise type from a coroutine function.
*/
Expand Down
Loading

0 comments on commit 28a859b

Please sign in to comment.