Skip to content

Commit

Permalink
Fix data race in task_manager::notify_horizon_executed assertion
Browse files Browse the repository at this point in the history
  • Loading branch information
fknorr committed Jan 3, 2022
1 parent 5f99f3b commit f641bcb
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
3 changes: 2 additions & 1 deletion include/task_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ namespace detail {
// The latest horizon task created. Will be applied as last writer once the next horizon is created.
task* current_horizon_task = nullptr;

// Queue of horizon tasks for which the associated commands were executed
// Queue of horizon tasks for which the associated commands were executed.
// Only accessed in task_manager::notify_horizon_executed, which is always called from the executor thread - no locking needed.
std::queue<task_id> executed_horizons;
// marker task id for "nothing to delete" - we can safely use 0 here
static constexpr task_id nothing_to_delete = 0;
Expand Down
12 changes: 9 additions & 3 deletions src/task_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,19 @@ namespace detail {
}

void task_manager::notify_horizon_executed(task_id tid) {
assert(task_map.at(tid)->get_type() == task_type::HORIZON);
#ifndef NDEBUG
{
std::lock_guard lock{task_mutex};
assert(task_map.count(tid) != 0);
assert(task_map.at(tid)->get_type() == task_type::HORIZON);
}
assert(executed_horizons.empty() || executed_horizons.back() != tid);
executed_horizons.push(tid);
#endif

executed_horizons.push(tid); // no locking needed - see definition
if(executed_horizons.size() >= horizon_deletion_lag) {
// actual cleanup happens on new task creation
horizon_task_id_for_deletion = executed_horizons.front();
horizon_task_id_for_deletion.store(executed_horizons.front()); // atomic
executed_horizons.pop();
}
}
Expand Down

0 comments on commit f641bcb

Please sign in to comment.