Skip to content

Commit 0ba7020

Browse files
committed
coro::task_container gc fix not completing coroutines
The coro::task_container::gc_internal function was deleting coroutines when marked as .done(), however there is some mechanism that rarely would cause the user_task coroutine to not actually execute. I'm still not sure exactly why this is the case but: 1) Simply disabling gc_internal() would stop the problem 2) Running gc_internal() and moving the coro::task to a 'dead' list still caused the issue. With these in mind I spent time re-reading the specification on the final_suspend and determined that coro::task_container should be a thing atomic counter to track the submitted coroutines and have them self delete. The self deletion is now done via a coro::detail::task_self_destroying coroutine type that takes advantage of the promise's final_suspend() not suspending. The spec states that if this doesn't suspend then the coroutine will call destroy() on itself. Closes #287
1 parent 26de94d commit 0ba7020

File tree

11 files changed

+260
-210
lines changed

11 files changed

+260
-210
lines changed

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ set(LIBCORO_SOURCE_FILES
7676
include/coro/concepts/promise.hpp
7777
include/coro/concepts/range_of.hpp
7878

79+
include/coro/detail/task_self_destroying.hpp / src/detail/task_self_destroying.cpp
7980
include/coro/detail/void_value.hpp
8081

8182
include/coro/attribute.hpp

examples/coro_task_container.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ int main()
5555
tc.start(serve_client(std::move(client)));
5656

5757
// Wait for all clients to complete before shutting down the tcp::server.
58-
co_await tc.garbage_collect_and_yield_until_empty();
58+
co_await tc.yield_until_empty();
5959
co_return;
6060
};
6161

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
#pragma once
2+
3+
#include <atomic>
4+
#include <coroutine>
5+
#include <cstdint>
6+
7+
namespace coro::detail
8+
{
9+
10+
class task_self_destroying;
11+
12+
class promise_self_destroying
13+
{
14+
public:
15+
promise_self_destroying();
16+
~promise_self_destroying();
17+
18+
promise_self_destroying(const promise_self_destroying&) = delete;
19+
promise_self_destroying(promise_self_destroying&&);
20+
auto operator=(const promise_self_destroying&) -> promise_self_destroying& = delete;
21+
auto operator=(promise_self_destroying&&) -> promise_self_destroying&;
22+
23+
auto get_return_object() -> task_self_destroying;
24+
auto initial_suspend() -> std::suspend_always;
25+
auto final_suspend() noexcept -> std::suspend_never;
26+
auto return_void() noexcept -> void;
27+
auto unhandled_exception() -> void;
28+
29+
auto task_container_size(std::atomic<std::size_t>& task_container_size) -> void;
30+
/**
31+
* The coro::task_container<executor_t> m_size member to decrement upon the coroutine completing.
32+
*/
33+
private:
34+
std::atomic<std::size_t>* m_task_container_size{nullptr};
35+
};
36+
37+
/**
38+
* This task will self delete upon completing. This is useful for usecase that the lifetime of the
39+
* coroutine cannot be determined and it needs to 'self' delete. This is achieved by returning
40+
* std::suspend_never from the promise::final_suspend which then based on the spec tells the
41+
* coroutine to destroy itself. This means any classes that use this task cannot have owning
42+
* pointers or relationships to this class and must not use it past its completion.
43+
*
44+
* This class is currently only used by coro::task_container<executor_t> and will decrement its
45+
* m_size internal count when the coroutine completes.
46+
*/
47+
class task_self_destroying
48+
{
49+
public:
50+
using promise_type = promise_self_destroying;
51+
52+
explicit task_self_destroying(promise_self_destroying& promise);
53+
~task_self_destroying();
54+
55+
task_self_destroying(const task_self_destroying&) = delete;
56+
task_self_destroying(task_self_destroying&&);
57+
auto operator=(const task_self_destroying&) -> task_self_destroying& = delete;
58+
auto operator=(task_self_destroying&&) -> task_self_destroying&;
59+
60+
auto promise() -> promise_self_destroying& { return *m_promise; }
61+
auto handle() -> std::coroutine_handle<promise_self_destroying> { return std::coroutine_handle<promise_self_destroying>::from_promise(*m_promise); }
62+
private:
63+
promise_self_destroying* m_promise{nullptr};
64+
};
65+
66+
} // namespace coro::detail

include/coro/io_scheduler.hpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ class io_scheduler : public std::enable_shared_from_this<io_scheduler>
178178
* @param task The task to execute on this io_scheduler. It's lifetime ownership will be transferred
179179
* to this io_scheduler.
180180
*/
181-
auto schedule(coro::task<void>&& task) -> void;
181+
auto schedule(coro::task<void>&& task) -> bool;
182182

183183
/**
184184
* Schedules the current task to run after the given amount of time has elapsed.
@@ -247,7 +247,7 @@ class io_scheduler : public std::enable_shared_from_this<io_scheduler>
247247
*/
248248
auto resume(std::coroutine_handle<> handle) -> bool
249249
{
250-
if (handle == nullptr)
250+
if (handle == nullptr || handle.done())
251251
{
252252
return false;
253253
}
@@ -259,6 +259,7 @@ class io_scheduler : public std::enable_shared_from_this<io_scheduler>
259259

260260
if (m_opts.execution_strategy == execution_strategy_t::process_tasks_inline)
261261
{
262+
m_size.fetch_add(1, std::memory_order::release);
262263
{
263264
std::scoped_lock lk{m_scheduled_tasks_mutex};
264265
m_scheduled_tasks.emplace_back(handle);
@@ -309,13 +310,6 @@ class io_scheduler : public std::enable_shared_from_this<io_scheduler>
309310
*/
310311
auto shutdown() noexcept -> void;
311312

312-
/**
313-
* Scans for completed coroutines and destroys them freeing up resources. This is also done on starting
314-
* new tasks but this allows the user to cleanup resources manually. One usage might be making sure fds
315-
* are cleaned up as soon as possible.
316-
*/
317-
auto garbage_collect() noexcept -> void;
318-
319313
private:
320314
/// The configuration options.
321315
options m_opts;

0 commit comments

Comments
 (0)