Skip to content

Commit

Permalink
Adjust when task_container's user_task is deleted
Browse files Browse the repository at this point in the history
It is now deleted inline in make_user_task so any destructors that get
invoked that possibly schedule more coroutines do not cause a deadlock
  • Loading branch information
jbaldwin committed Jul 2, 2024
1 parent 567f4a8 commit becd591
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 19 deletions.
3 changes: 3 additions & 0 deletions include/coro/concepts/executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ concept executor = requires(type t, std::coroutine_handle<> c)
{ t.schedule() } -> coro::concepts::awaiter;
{ t.yield() } -> coro::concepts::awaiter;
{ t.resume(c) } -> std::same_as<bool>;
{ t.size() } -> std::same_as<std::size_t>;
{ t.empty() } -> std::same_as<bool>;
{ t.shutdown() } -> std::same_as<void>;
};

#ifdef LIBCORO_FEATURE_NETWORKING
Expand Down
42 changes: 23 additions & 19 deletions include/coro/task_container.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <memory>
#include <mutex>
#include <queue>
#include <thread>
#include <vector>

namespace coro
Expand Down Expand Up @@ -78,25 +79,25 @@ class task_container
{
m_size.fetch_add(1, std::memory_order::relaxed);

std::unique_lock lk{m_mutex};

if (cleanup == garbage_collect_t::yes)
std::size_t index{};
{
gc_internal();
}
std::unique_lock lk{m_mutex};

// Only grow if completely full and attempting to add more.
if (m_free_task_indices.empty())
{
grow();
}
if (cleanup == garbage_collect_t::yes)
{
gc_internal();
}

// Reserve a free task index
std::size_t index = m_free_task_indices.front();
m_free_task_indices.pop();
// Only grow if completely full and attempting to add more.
if (m_free_task_indices.empty())
{
grow();
}

// We've reserved the slot, we can release the lock.
lk.unlock();
// Reserve a free task index
index = m_free_task_indices.front();
m_free_task_indices.pop();
}

// Store the task inside a cleanup task for self deletion.
m_tasks[index] = make_cleanup_task(std::move(user_task), index);
Expand All @@ -106,7 +107,7 @@ class task_container
}

/**
* Garbage collects any tasks that are marked as deleted. This frees up space to be re-used by
* Garbage collects any tasks that are marked as deleted. This frees up space to be re-used by
* the task container for newly stored tasks.
* @return The number of tasks that were deleted.
*/
Expand Down Expand Up @@ -182,7 +183,7 @@ class task_container
pos++;
continue;
}
// Destroy the cleanup task and the user task.
// Destroy the cleanup task.
m_tasks[*pos].destroy();
// Put the deleted position at the end of the free indexes list.
m_free_task_indices.emplace(*pos);
Expand Down Expand Up @@ -231,9 +232,12 @@ class task_container
std::cerr << "coro::task_container user_task had unhandle exception, not derived from std::exception.\n";
}

// Destroy the user task since it is complete. This is important to do so outside the lock
// since the user could schedule a new task from the destructor (tls::client does this interanlly)
// causing a deadlock.
user_task.destroy();

{
// This scope is required around this lock otherwise if this task on destruction schedules a new task it
// can cause a deadlock, notably tls::client schedules a task to cleanup tls resources.
std::scoped_lock lk{m_mutex};
m_tasks_to_delete.emplace_back(index);
}
Expand Down

0 comments on commit becd591

Please sign in to comment.