Skip to content

Commit

Permalink
future: pass future pointer to callback directly
Browse files Browse the repository at this point in the history
Before this PR, the pointer was obtained from a valid
reference &CassFuture, which is totally fine.

However, I want to reduce the ways one can obtain such pointer.
For ArcFFI (shared pointers), I want them to be obtainable only in two ways:
- `ArcFFI::as_ptr()` which accepts an &Arc
- from the user, as a function parameter

This way, we are guaranteed that the pointer comes from a valid Arc allocation
(unless user provided pointer to some garbage, but there is no much we can do about it).
If we assume that user provides a pointer returned from some prior call to
API, we are guaranteed that it is valid, and comes from an Arc allocation (or is null).

I don't want to allow ArcFFI api to create a pointer from a refernce, to prevent
creating a pointer, from example from stack allocated object:
```
let future = CassFuture { ... };
let future_ptr = ArcFFI::as_ptr(&future);
```

This commit may not make much sense now, but all should be clear once
I introduce traits restricting the pointer types later in this PR.
  • Loading branch information
muzarski committed Dec 2, 2024
1 parent b03d735 commit e5efd2d
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions scylla-rust-wrapper/src/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ struct BoundCallback {
unsafe impl Send for BoundCallback {}

impl BoundCallback {
fn invoke(self, fut: &CassFuture) {
fn invoke(self, fut_ptr: *const CassFuture) {
unsafe {
self.cb.unwrap()(fut as *const CassFuture, self.data);
self.cb.unwrap()(fut_ptr, self.data);
}
}
}
Expand Down Expand Up @@ -96,7 +96,8 @@ impl CassFuture {
guard.callback.take()
};
if let Some(bound_cb) = maybe_cb {
bound_cb.invoke(cass_fut_clone.as_ref());
let fut_ptr = ArcFFI::as_ptr(&cass_fut_clone);
bound_cb.invoke(fut_ptr);
}

cass_fut_clone.wait_for_value.notify_all();
Expand Down Expand Up @@ -258,7 +259,12 @@ impl CassFuture {
}
}

pub fn set_callback(&self, cb: CassFutureCallback, data: *mut c_void) -> CassError {
pub fn set_callback(
&self,
self_ptr: *const CassFuture,
cb: CassFutureCallback,
data: *mut c_void,
) -> CassError {
let mut lock = self.state.lock().unwrap();
if lock.callback.is_some() {
// Another callback has been already set
Expand All @@ -268,7 +274,7 @@ impl CassFuture {
if lock.value.is_some() {
// The value is already available, we need to call the callback ourselves
mem::drop(lock);
bound_cb.invoke(self);
bound_cb.invoke(self_ptr);
return CassError::CASS_OK;
}
// Store the callback
Expand All @@ -293,7 +299,7 @@ pub unsafe extern "C" fn cass_future_set_callback(
callback: CassFutureCallback,
data: *mut ::std::os::raw::c_void,
) -> CassError {
ArcFFI::as_ref(future_raw).set_callback(callback, data)
ArcFFI::as_ref(future_raw).set_callback(future_raw, callback, data)
}

#[no_mangle]
Expand Down

0 comments on commit e5efd2d

Please sign in to comment.