Skip to content

Commit

Permalink
SE: Fix possible race condition
Browse files Browse the repository at this point in the history
This commit fixes a panic in
`scylla::transport::speculative_execution::execute`.

It is possible for all speculative fibers to finish without returning
from the function. In that situation `select!` macro will panic because
all futures passed to it are completed.

Consider the scenario where all executions return BrokenConnection:
we will only assign to last_error and never return.
There are more ways that this bug can happen, but this is the simplest
one to explain and reproduce.

This commit fixes the bug by simplifying the logic inside the function.
The new logic always checks if the resolved task is the last one, and if
so it returns, so it's not possible for executions to be exhausted
when calling `select!`.
  • Loading branch information
Lorak-mmk committed Oct 9, 2024
1 parent ea7c464 commit c5fb18d
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions scylla/src/transport/speculative_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,21 +127,17 @@ where
}
}
res = async_tasks.select_next_some() => {
match res {
Some(r) => {
if !can_be_ignored(&r) {
return r;
} else {
last_error = Some(r)
}
},
None => {
if async_tasks.is_empty() && retries_remaining == 0 {
return last_error.unwrap_or({
Err(EMPTY_PLAN_ERROR)
});
}
},
if let Some(r) = res {
if !can_be_ignored(&r) {
return r;
} else {
last_error = Some(r)
}
}
if async_tasks.is_empty() && retries_remaining == 0 {
return last_error.unwrap_or({
Err(EMPTY_PLAN_ERROR)
});
}
}
}
Expand Down

0 comments on commit c5fb18d

Please sign in to comment.