Skip to content

Commit

Permalink
argconv: make FFI implementation mutually exclusive
Browse files Browse the repository at this point in the history
Disallow implementing two different APIs for specific type.
  • Loading branch information
muzarski committed Dec 17, 2024
1 parent 8208ee4 commit 66b329e
Show file tree
Hide file tree
Showing 19 changed files with 160 additions and 30 deletions.
80 changes: 77 additions & 3 deletions scylla-rust-wrapper/src/argconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,12 +498,18 @@ impl<T: Sized> CassBorrowedPtr<T> {
}
}

mod own_sealed {
pub trait ExclusiveSealed {}
pub trait SharedSealed {}
pub trait BorrowedSealed {}
}

/// Defines a pointer manipulation API for non-shared heap-allocated data.
///
/// Implement this trait for types that are allocated by the driver via [`Box::new`],
/// and then returned to the user as a pointer. The user is responsible for freeing
/// the memory associated with the pointer using corresponding driver's API function.
pub trait BoxFFI: Sized {
pub trait BoxFFI: Sized + own_sealed::ExclusiveSealed {
/// Consumes the Box and returns a pointer with exclusive ownership.
fn into_ptr<M: Mutability>(self: Box<Self>) -> CassExclusivePtr<Self, M> {
CassExclusivePtr::from_box(self)
Expand Down Expand Up @@ -552,7 +558,7 @@ pub trait BoxFFI: Sized {
/// The data should be allocated via [`Arc::new`], and then returned to the user as a pointer.
/// The user is responsible for freeing the memory associated
/// with the pointer using corresponding driver's API function.
pub trait ArcFFI: Sized {
pub trait ArcFFI: Sized + own_sealed::SharedSealed {
/// Creates a pointer from a valid reference to Arc-allocated data.
/// The pointer is [`Borrowed`].
fn as_ptr(self: &Arc<Self>) -> CassSharedBorrowedPtr<Self> {
Expand Down Expand Up @@ -618,7 +624,7 @@ pub trait ArcFFI: Sized {
/// For example: lifetime of CassRow is bound by the lifetime of CassResult.
/// There is no API function that frees the CassRow. It should be automatically
/// freed when user calls cass_result_free.
pub trait RefFFI: Sized {
pub trait RefFFI: Sized + own_sealed::BorrowedSealed {
/// Creates a borrowed pointer from a valid reference.
fn as_ptr(&self) -> CassBorrowedPtr<Self> {
CassBorrowedPtr::from_ref(self)
Expand Down Expand Up @@ -646,3 +652,71 @@ pub trait RefFFI: Sized {
ptr.is_null()
}
}

/// This trait should be implemented for types that are passed between
/// C and Rust API. We currently distinguish 3 kinds of implementors,
/// wrt. the ownership. The implementor should pick one of the 3 ownership
/// kinds as the associated type:
/// - [`OwnershipExclusive`]
/// - [`OwnershipShared`]
/// - [`OwnershipBorrowed`]
#[allow(clippy::upper_case_acronyms)]
pub trait FFI {
type Ownership;
}

/// Represents types with an exclusive ownership.
///
/// Use this associated type for implementors that require:
/// - [`CassExclusivePtr`] manipulation via [`BoxFFI`]
/// - exclusive ownership of the corresponding object
/// - potential mutability of the corresponding object
/// - manual memory freeing
///
/// C API user should be responsible for freeing associated memory manually
/// via corresponding API call.
///
/// An example of such implementor would be [`CassCluster`](crate::cluster::CassCluster):
/// - it is allocated on the heap via [`Box::new`]
/// - user is the exclusive owner of the CassCluster object
/// - there is no API to increase a reference count of CassCluster object
/// - CassCluster is mutable via some API methods (`cass_cluster_set_*`)
/// - user is responsible for freeing the associated memory (`cass_cluster_free`)
pub struct OwnershipExclusive;
impl<T> own_sealed::ExclusiveSealed for T where T: FFI<Ownership = OwnershipExclusive> {}
impl<T> BoxFFI for T where T: FFI<Ownership = OwnershipExclusive> {}

/// Represents types with a shared ownership.
///
/// Use this associated type for implementors that require:
/// - [`CassSharedPtr`] manipulation via [`ArcFFI`]
/// - shared ownership of the corresponding object
/// - manual memory freeing
///
/// C API user should be responsible for freeing (decreasing reference count of)
/// associated memory manually via corresponding API call.
///
/// An example of such implementor would be [`CassDataType`](crate::cass_types::CassDataType):
/// - it is allocated on the heap via [`Arc::new`]
/// - there are multiple owners of the shared CassDataType object
/// - some API functions require to increase a reference count of the object
/// - user is responsible for freeing (decreasing RC of) the associated memory (`cass_data_type_free`)
pub struct OwnershipShared;
impl<T> own_sealed::SharedSealed for T where T: FFI<Ownership = OwnershipShared> {}
impl<T> ArcFFI for T where T: FFI<Ownership = OwnershipShared> {}

/// Represents borrowed types.
///
/// Use this associated type for implementors that do not require any assumptions
/// about the pointer type (apart from validity).
/// The implementation will enable [`CassBorrowedPtr`] manipulation via [`RefFFI`]
///
/// C API user is not responsible for freeing associated memory manually. The memory
/// should be freed automatically, when the owner is being dropped.
///
/// An example of such implementor would be [`CassRow`](crate::query_result::CassRow):
/// - its lifetime is tied to the lifetime of CassResult
/// - user only "borrows" the pointer - he is not responsible for freeing the memory
pub struct OwnershipBorrowed;
impl<T> own_sealed::BorrowedSealed for T where T: FFI<Ownership = OwnershipBorrowed> {}
impl<T> RefFFI for T where T: FFI<Ownership = OwnershipBorrowed> {}
5 changes: 4 additions & 1 deletion scylla-rust-wrapper/src/batch.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::argconv::{
ArcFFI, BoxFFI, CassExclusiveConstPtr, CassExclusiveMutPtr, CassSharedBorrowedPtr,
OwnershipExclusive, FFI,
};
use crate::cass_error::CassError;
use crate::cass_types::CassConsistency;
Expand All @@ -21,7 +22,9 @@ pub struct CassBatch {
pub(crate) exec_profile: Option<PerStatementExecProfile>,
}

impl BoxFFI for CassBatch {}
impl FFI for CassBatch {
type Ownership = OwnershipExclusive;
}

#[derive(Clone)]
pub struct CassBatchState {
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/cass_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,9 @@ pub enum CassDataTypeInner {
Custom(String),
}

impl ArcFFI for CassDataType {}
impl FFI for CassDataType {
type Ownership = OwnershipShared;
}

impl CassDataTypeInner {
/// Checks for equality during typechecks.
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ impl CassCluster {
}
}

impl BoxFFI for CassCluster {}
impl FFI for CassCluster {
type Ownership = OwnershipExclusive;
}

pub struct CassCustomPayload;

Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ pub struct CassCollection {
pub items: Vec<CassCqlValue>,
}

impl BoxFFI for CassCollection {}
impl FFI for CassCollection {
type Ownership = OwnershipExclusive;
}

impl CassCollection {
fn typecheck_on_append(&self, value: &Option<CassCqlValue>) -> CassError {
Expand Down
5 changes: 4 additions & 1 deletion scylla-rust-wrapper/src/exec_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use scylla::statement::Consistency;

use crate::argconv::{
ptr_to_cstr_n, strlen, ArcFFI, BoxFFI, CassExclusiveMutPtr, CassSharedBorrowedPtr,
OwnershipExclusive, FFI,
};
use crate::batch::CassBatch;
use crate::cass_error::CassError;
Expand All @@ -39,7 +40,9 @@ pub struct CassExecProfile {
load_balancing_config: LoadBalancingConfig,
}

impl BoxFFI for CassExecProfile {}
impl FFI for CassExecProfile {
type Ownership = OwnershipExclusive;
}

impl CassExecProfile {
fn new() -> Self {
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ pub struct CassFuture {
wait_for_value: Condvar,
}

impl ArcFFI for CassFuture {}
impl FFI for CassFuture {
type Ownership = OwnershipShared;
}

/// An error that can appear during `cass_future_wait_timed`.
enum FutureError {
Expand Down
8 changes: 6 additions & 2 deletions scylla-rust-wrapper/src/logging.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::argconv::{arr_to_cstr, ptr_to_cstr, str_to_arr, CassBorrowedPtr, RefFFI};
use crate::argconv::{
arr_to_cstr, ptr_to_cstr, str_to_arr, CassBorrowedPtr, OwnershipBorrowed, RefFFI, FFI,
};
use crate::cass_log_types::{CassLogLevel, CassLogMessage};
use crate::types::size_t;
use crate::LOGGER;
Expand All @@ -14,7 +16,9 @@ use tracing_subscriber::layer::Context;
use tracing_subscriber::prelude::*;
use tracing_subscriber::Layer;

impl RefFFI for CassLogMessage {}
impl FFI for CassLogMessage {
type Ownership = OwnershipBorrowed;
}

pub type CassLogCallback =
Option<unsafe extern "C" fn(message: CassBorrowedPtr<CassLogMessage>, data: *mut c_void)>;
Expand Down
20 changes: 15 additions & 5 deletions scylla-rust-wrapper/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ pub struct CassSchemaMeta {
pub keyspaces: HashMap<String, CassKeyspaceMeta>,
}

impl BoxFFI for CassSchemaMeta {}
impl FFI for CassSchemaMeta {
type Ownership = OwnershipExclusive;
}

pub struct CassKeyspaceMeta {
pub name: String,
Expand All @@ -25,7 +27,9 @@ pub struct CassKeyspaceMeta {
}

// Owned by CassSchemaMeta
impl RefFFI for CassKeyspaceMeta {}
impl FFI for CassKeyspaceMeta {
type Ownership = OwnershipBorrowed;
}

pub struct CassTableMeta {
pub name: String,
Expand All @@ -38,7 +42,9 @@ pub struct CassTableMeta {
// Either:
// - owned by CassMaterializedViewMeta - won't be given to user
// - Owned by CassKeyspaceMeta (in Arc), referenced (Weak) by CassMaterializedViewMeta
impl RefFFI for CassTableMeta {}
impl FFI for CassTableMeta {
type Ownership = OwnershipBorrowed;
}

pub struct CassMaterializedViewMeta {
pub name: String,
Expand All @@ -47,7 +53,9 @@ pub struct CassMaterializedViewMeta {
}

// Shared ownership by CassKeyspaceMeta and CassTableMeta
impl RefFFI for CassMaterializedViewMeta {}
impl FFI for CassMaterializedViewMeta {
type Ownership = OwnershipBorrowed;
}

pub struct CassColumnMeta {
pub name: String,
Expand All @@ -56,7 +64,9 @@ pub struct CassColumnMeta {
}

// Owned by CassTableMeta
impl RefFFI for CassColumnMeta {}
impl FFI for CassColumnMeta {
type Ownership = OwnershipBorrowed;
}

pub unsafe fn create_table_metadata(
keyspace_name: &str,
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/prepared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ impl CassPrepared {
}
}

impl ArcFFI for CassPrepared {}
impl FFI for CassPrepared {
type Ownership = OwnershipShared;
}

#[no_mangle]
pub unsafe extern "C" fn cass_prepared_free(prepared_raw: CassSharedOwnedPtr<CassPrepared>) {
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/query_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ pub enum CassErrorResult {
Deserialization(#[from] DeserializationError),
}

impl ArcFFI for CassErrorResult {}
impl FFI for CassErrorResult {
type Ownership = OwnershipShared;
}

impl From<Consistency> for CassConsistency {
fn from(c: Consistency) -> CassConsistency {
Expand Down
16 changes: 12 additions & 4 deletions scylla-rust-wrapper/src/query_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ impl CassResult {
}
}

impl ArcFFI for CassResult {}
impl FFI for CassResult {
type Ownership = OwnershipShared;
}

#[derive(Debug)]
pub struct CassResultMetadata {
Expand Down Expand Up @@ -149,7 +151,9 @@ pub struct CassRow {
pub result_metadata: Arc<CassResultMetadata>,
}

impl RefFFI for CassRow {}
impl FFI for CassRow {
type Ownership = OwnershipBorrowed;
}

pub fn create_cass_rows_from_rows(
rows: Vec<Row>,
Expand Down Expand Up @@ -185,7 +189,9 @@ pub struct CassValue {
pub value_type: Arc<CassDataType>,
}

impl RefFFI for CassValue {}
impl FFI for CassValue {
type Ownership = OwnershipBorrowed;
}

fn create_cass_row_columns(row: Row, metadata: &Arc<CassResultMetadata>) -> Vec<CassValue> {
row.columns
Expand Down Expand Up @@ -367,7 +373,9 @@ pub enum CassIterator {
CassViewMetaIterator(CassViewMetaIterator),
}

impl BoxFFI for CassIterator {}
impl FFI for CassIterator {
type Ownership = OwnershipExclusive;
}

#[no_mangle]
pub unsafe extern "C" fn cass_iterator_free(iterator: CassExclusiveMutPtr<CassIterator>) {
Expand Down
6 changes: 4 additions & 2 deletions scylla-rust-wrapper/src/retry_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use scylla::retry_policy::{DefaultRetryPolicy, FallthroughRetryPolicy};
use scylla::transport::downgrading_consistency_retry_policy::DowngradingConsistencyRetryPolicy;
use std::sync::Arc;

use crate::argconv::{ArcFFI, CassSharedOwnedPtr};
use crate::argconv::{ArcFFI, CassSharedOwnedPtr, OwnershipShared, FFI};

pub enum RetryPolicy {
DefaultRetryPolicy(Arc<DefaultRetryPolicy>),
Expand All @@ -12,7 +12,9 @@ pub enum RetryPolicy {

pub type CassRetryPolicy = RetryPolicy;

impl ArcFFI for CassRetryPolicy {}
impl FFI for CassRetryPolicy {
type Ownership = OwnershipShared;
}

#[no_mangle]
pub extern "C" fn cass_retry_policy_default_new() -> CassSharedOwnedPtr<CassRetryPolicy> {
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ impl CassSessionInner {

pub type CassSession = RwLock<Option<CassSessionInner>>;

impl ArcFFI for CassSession {}
impl FFI for CassSession {
type Ownership = OwnershipShared;
}

#[no_mangle]
pub unsafe extern "C" fn cass_session_new() -> CassSharedOwnedPtr<CassSession> {
Expand Down
6 changes: 5 additions & 1 deletion scylla-rust-wrapper/src/ssl.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::argconv::ArcFFI;
use crate::argconv::CassSharedBorrowedPtr;
use crate::argconv::CassSharedOwnedPtr;
use crate::argconv::OwnershipShared;
use crate::argconv::FFI;
use crate::cass_error::CassError;
use crate::types::size_t;
use libc::{c_int, strlen};
Expand All @@ -21,7 +23,9 @@ pub struct CassSsl {
pub(crate) trusted_store: *mut X509_STORE,
}

impl ArcFFI for CassSsl {}
impl FFI for CassSsl {
type Ownership = OwnershipShared;
}

pub const CASS_SSL_VERIFY_NONE: i32 = 0x00;
pub const CASS_SSL_VERIFY_PEER_CERT: i32 = 0x01;
Expand Down
4 changes: 3 additions & 1 deletion scylla-rust-wrapper/src/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,9 @@ pub struct CassStatement {
pub(crate) exec_profile: Option<PerStatementExecProfile>,
}

impl BoxFFI for CassStatement {}
impl FFI for CassStatement {
type Ownership = OwnershipExclusive;
}

impl CassStatement {
fn bind_cql_value(&mut self, index: usize, value: Option<CassCqlValue>) -> CassError {
Expand Down
Loading

0 comments on commit 66b329e

Please sign in to comment.