diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 00000000..e002f4dc --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,56 @@ +language: en + +early_access: false + +chat: + auto_reply: true + +reviews: + auto_review: + enabled: true + ignore_title_keywords: + - "WIP" + drafts: false + base_branches: + - master + - develop + + high_level_summary: true + + # Generate sequence diagrams for complex code flows + sequence_diagrams: true + + poem: true + review_status: true + collapse_walkthrough: true + changed_files_summary: true + request_changes_workflow: false + + pre_merge_checks: + description: + mode: warning # Options: off, warning, error + docstrings: + mode: off # Disabled: cannot exclude detail namespaces from coverage + + path_filters: + - "!**/bench/**" + - "!**/build/**" + - "!**/context/**" + - "!**/doc/**" + - "!**/meta/**" + - "!**/papers/**" + - "!**/test/**" + + # Custom review instructions for specific file patterns + path_instructions: + - path: "**/*.{cpp,hpp}" + instructions: | + Documentation Best Practices + - The top of the file after the includes, put a nice /* */ section + which gives a high level overview of how the implementation works. + - Single line // comments are to be used sparingly and judiciously + which explain the why (not the what or how) when it is non-obvious. + - Docstrings are required for all classes in public headers in + non detail namespaces. They are used for generating the Documentation + website. Please give warnings for any class or function that does + not have a docstring or has an insufficient docstring. diff --git a/include/boost/capy/ex/run_async.hpp b/include/boost/capy/ex/run_async.hpp index fae95578..eb85fc6b 100644 --- a/include/boost/capy/ex/run_async.hpp +++ b/include/boost/capy/ex/run_async.hpp @@ -200,7 +200,7 @@ struct trampoline std::coroutine_handle<> task_h_; // Constructor receives coroutine parameters by lvalue reference - promise_type(Ex ex, Handlers h) + promise_type(Ex &ex, Handlers &h) : ex_(std::move(ex)) , handlers_(std::move(h)) { diff --git a/include/boost/capy/io_awaitable.hpp b/include/boost/capy/io_awaitable.hpp index 27648da6..93d6aeef 100644 --- a/include/boost/capy/io_awaitable.hpp +++ b/include/boost/capy/io_awaitable.hpp @@ -232,7 +232,7 @@ concept IoAwaitableTask = { p.set_continuation(cont, ex) } noexcept; { cp.executor() } noexcept -> std::same_as; { cp.stop_token() } noexcept -> std::same_as; - { cp.complete() } noexcept -> std::same_as; + { p.complete() } noexcept -> std::same_as; }; /** Concept for launchable I/O task types. @@ -382,7 +382,7 @@ class io_awaitable_support { executor_ref executor_; std::stop_token stop_token_; - coro cont_; + coro cont_{std::noop_coroutine()}; executor_ref caller_ex_; public: @@ -413,13 +413,13 @@ class io_awaitable_support @return A coroutine handle for symmetric transfer. */ - coro complete() const noexcept + coro complete() noexcept { if(!cont_) return std::noop_coroutine(); if(executor_ == caller_ex_) - return cont_; - return caller_ex_.dispatch(cont_); + return std::exchange(cont_, std::noop_coroutine()); + return caller_ex_.dispatch(std::exchange(cont_, std::noop_coroutine())); } /** Store a stop token for later retrieval. @@ -543,6 +543,13 @@ class io_awaitable_support std::forward(t)); } } + + constexpr io_awaitable_support() noexcept = default; + io_awaitable_support(const io_awaitable_support & ) = delete; + ~io_awaitable_support() + { + cont_.destroy(); + } }; } // namespace capy diff --git a/test/unit/task.cpp b/test/unit/task.cpp index 9c81953c..6afe8b72 100644 --- a/test/unit/task.cpp +++ b/test/unit/task.cpp @@ -15,6 +15,8 @@ #include "test_helpers.hpp" #include +#include +#include #include #include #include @@ -1074,6 +1076,90 @@ struct task_test BOOST_TEST(all_same); } + struct tear_down_t + { + bool *torn_down; + tear_down_t(bool &torn_down) : torn_down(&torn_down) {} + tear_down_t(tear_down_t && rhs) noexcept : torn_down(std::exchange(rhs.torn_down, nullptr)) {} + ~tear_down_t() + { + if (torn_down) + *torn_down = true; + } + }; + + void testTearDown() + { + bool torn_down = false; + + + auto l =[](tear_down_t ) -> capy::task {co_return ;}; + + { + auto t = l(torn_down); + BOOST_TEST(!torn_down); + } + + BOOST_TEST(torn_down); + } + + void testDelete() + { + bool td1 = false, td2 = false; + { + auto l = []() -> capy::task + { + co_await self_destroy_awaitable{}; + }; + int dispatch_count = 0; + test_executor ex(dispatch_count); + + run_async(ex, + [t = tear_down_t(td1)] { BOOST_TEST(false); }, + [t = tear_down_t(td2)](std::exception_ptr) {BOOST_TEST(false);})(l()); + + BOOST_TEST(dispatch_count == 1); // async_run dispatches once to start the test. + } + // CHECK if the handlers got destroyed - since we're doing a hard shutdown here, it's ok if they were to live on in the executor. + BOOST_TEST(td1 == true); + BOOST_TEST(td2 == true); + } + + void testStop() + { + + std::atomic state = 0; + + auto l = [&]() -> capy::task + { + struct scope_check + { + std::atomic &state; + ~scope_check() { BOOST_TEST(state == 2);} + } sc{state}; + + state = 1; + co_await stop_only_awaitable{}; + state = 2; + }; + + + { + std::jthread jt( + [&](std::stop_token st) + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + run_async(ex, st)(l()); + } + ); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + BOOST_TEST(state == 1); + } + + BOOST_TEST(state == 2); + } + void run() { @@ -1111,6 +1197,10 @@ struct task_test testGetStopTokenPropagation(); testGetStopTokenInLoop(); testGetStopTokenMultipleCalls(); + + testTearDown(); + testDelete(); + testStop(); } }; diff --git a/test/unit/test_helpers.hpp b/test/unit/test_helpers.hpp index 5320587e..f48fda93 100644 --- a/test/unit/test_helpers.hpp +++ b/test/unit/test_helpers.hpp @@ -12,6 +12,7 @@ // Trick boostdep into requiring URL // since we need it for the unit tests +#include #ifdef BOOST_CAPY_BOOSTDEP #include #endif @@ -20,6 +21,9 @@ #include #include +#include +#include + #include "test_suite.hpp" namespace boost { @@ -108,6 +112,48 @@ struct test_executor static_assert(Executor); + +struct self_destroy_awaitable +{ + bool await_ready() {return false;} + + template + std::coroutine_handle<> await_suspend(std::coroutine_handle<> h, + const Executor &, + std::stop_token) + { + // one wouldn't expect this to happen, but it should not cause UB + h.destroy(); + return std::noop_coroutine(); + } + void await_resume() {} +}; + + +// test awaitable that must be stopped in order to resume +struct stop_only_awaitable +{ + stop_only_awaitable() noexcept = default; + stop_only_awaitable(stop_only_awaitable && ) noexcept {} + + std::optional>> stop_cb; + + bool await_ready() {return false;} + + template + std::coroutine_handle<> await_suspend(std::coroutine_handle<> h, + const Executor &, + std::stop_token tk) + { + if (tk.stop_requested()) + return h; + stop_cb.emplace(tk, h); + return std::noop_coroutine(); + } + void await_resume() {} +}; + + } // capy } // boost