Skip to content

Commit 18bd75c

Browse files
authored
future: Fix double callback execution (#468)
`ResolvableFuture::set_callback` first set a callback under lock, released the lock, and then checked if result is set. If it is, then it invoked the callback. The other path for callback calling is in future execution: when setting the result (under lock) new_from_future also reads a callback. If it exsits, then it is called. the following race is possible: - `set_callback` sets callback, releases lock - `new_from_future` produces result, sees the callback, calls it - `set_callback` checks for the result, sees it exists, calls the callback. This results in double callback execution, potentially causing UB and other issues. This commit fixes that: in `set_callback`, while still under lock, we check if callback should be called. Then outside of lock we call it if needed. It is important to call the callback outside the lock, to avoid various deadlocks. Fixes: https://scylladb.atlassian.net/browse/CUSTOMER-409
2 parents 530d334 + f50548a commit 18bd75c

1 file changed

Lines changed: 5 additions & 3 deletions

File tree

scylla-rust-wrapper/src/future.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ impl ResolvableFuture {
424424
// Check if the callback is already set (in such case we must error out).
425425
// If it is not set, we store the callback in the state, so that no different
426426
// callback can be set.
427-
{
427+
let should_invoke_callback = {
428428
let mut lock = self.state.lock().unwrap();
429429
if lock.callback.is_some() {
430430
// Another callback has been already set
@@ -434,9 +434,11 @@ impl ResolvableFuture {
434434
// Store the callback, so that no other callback can be set from now on.
435435
// Rationale: only one callback can be set for the whole lifetime of a future.
436436
lock.callback = Some(bound_cb);
437-
}
438437

439-
if self.result.get().is_some() {
438+
self.result.get().is_some()
439+
};
440+
441+
if should_invoke_callback {
440442
// The value is already available, we need to call the callback ourselves
441443
bound_cb.invoke(self_ptr);
442444
return CassError::CASS_OK;

0 commit comments

Comments
 (0)