Skip to content

Commit ce3896f

Browse files
committed
argconv: make FFI implementation mutually exclusive
Disallow implementing two different APIs for specific type.
1 parent 0a9e955 commit ce3896f

20 files changed

+185
-46
lines changed

scylla-rust-wrapper/src/argconv.rs

Lines changed: 97 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -328,12 +328,18 @@ impl<T: Sized> CassPtr<'_, T, (Mut, CMut)> {
328328
}
329329
}
330330

331+
mod origin_sealed {
332+
pub trait FromBoxSealed {}
333+
pub trait FromArcSealed {}
334+
pub trait FromRefSealed {}
335+
}
336+
331337
/// Defines a pointer manipulation API for non-shared heap-allocated data.
332338
///
333339
/// Implement this trait for types that are allocated by the driver via [`Box::new`],
334340
/// and then returned to the user as a pointer. The user is responsible for freeing
335341
/// the memory associated with the pointer using corresponding driver's API function.
336-
pub trait BoxFFI: Sized {
342+
pub trait BoxFFI: Sized + origin_sealed::FromBoxSealed {
337343
/// Consumes the Box and returns a pointer with exclusive ownership.
338344
/// The pointer needs to be freed. See [`BoxFFI::free()`].
339345
fn into_ptr<M: Mutability, CM: CMutability>(
@@ -406,7 +412,7 @@ pub trait BoxFFI: Sized {
406412
/// The data should be allocated via [`Arc::new`], and then returned to the user as a pointer.
407413
/// The user is responsible for freeing the memory associated
408414
/// with the pointer using corresponding driver's API function.
409-
pub trait ArcFFI: Sized {
415+
pub trait ArcFFI: Sized + origin_sealed::FromArcSealed {
410416
/// Creates a pointer from a valid reference to Arc-allocated data.
411417
/// Holder of the pointer borrows the pointee.
412418
fn as_ptr<'a, CM: CMutability>(self: &'a Arc<Self>) -> CassPtr<'a, Self, (Const, CM)> {
@@ -496,7 +502,7 @@ pub trait ArcFFI: Sized {
496502
/// For example: lifetime of CassRow is bound by the lifetime of CassResult.
497503
/// There is no API function that frees the CassRow. It should be automatically
498504
/// freed when user calls cass_result_free.
499-
pub trait RefFFI: Sized {
505+
pub trait RefFFI: Sized + origin_sealed::FromRefSealed {
500506
/// Creates a borrowed pointer from a valid reference.
501507
#[allow(clippy::needless_lifetimes)]
502508
fn as_ptr<'a, CM: CMutability>(&'a self) -> CassPtr<'a, Self, (Const, CM)> {
@@ -542,11 +548,81 @@ pub trait RefFFI: Sized {
542548
}
543549
}
544550

551+
/// This trait should be implemented for types that are passed between
552+
/// C and Rust API. We currently distinguish 3 kinds of implementors,
553+
/// wrt. the origin of the pointer. The implementor should pick one of the 3 ownership
554+
/// kinds as the associated type:
555+
/// - [`FromBox`]
556+
/// - [`FromArc`]
557+
/// - [`FromRef`]
558+
#[allow(clippy::upper_case_acronyms)]
559+
pub trait FFI {
560+
type Origin;
561+
}
562+
563+
/// Represents types with an exclusive ownership.
564+
///
565+
/// Use this associated type for implementors that require:
566+
/// - owned exclusive pointer manipulation via [`BoxFFI`]
567+
/// - exclusive ownership of the corresponding object
568+
/// - potential mutability of the corresponding object
569+
/// - manual memory freeing
570+
///
571+
/// C API user should be responsible for freeing associated memory manually
572+
/// via corresponding API call.
573+
///
574+
/// An example of such implementor would be [`CassCluster`](crate::cluster::CassCluster):
575+
/// - it is allocated on the heap via [`Box::new`]
576+
/// - user is the exclusive owner of the CassCluster object
577+
/// - there is no API to increase a reference count of CassCluster object
578+
/// - CassCluster is mutable via some API methods (`cass_cluster_set_*`)
579+
/// - user is responsible for freeing the associated memory (`cass_cluster_free`)
580+
pub struct FromBox;
581+
impl<T> origin_sealed::FromBoxSealed for T where T: FFI<Origin = FromBox> {}
582+
impl<T> BoxFFI for T where T: FFI<Origin = FromBox> {}
583+
584+
/// Represents types with a shared ownership.
585+
///
586+
/// Use this associated type for implementors that require:
587+
/// - pointer with shared ownership manipulation via [`ArcFFI`]
588+
/// - shared ownership of the corresponding object
589+
/// - manual memory freeing
590+
///
591+
/// C API user should be responsible for freeing (decreasing reference count of)
592+
/// associated memory manually via corresponding API call.
593+
///
594+
/// An example of such implementor would be [`CassDataType`](crate::cass_types::CassDataType):
595+
/// - it is allocated on the heap via [`Arc::new`]
596+
/// - there are multiple owners of the shared CassDataType object
597+
/// - some API functions require to increase a reference count of the object
598+
/// - user is responsible for freeing (decreasing RC of) the associated memory (`cass_data_type_free`)
599+
pub struct FromArc;
600+
impl<T> origin_sealed::FromArcSealed for T where T: FFI<Origin = FromArc> {}
601+
impl<T> ArcFFI for T where T: FFI<Origin = FromArc> {}
602+
603+
/// Represents borrowed types.
604+
///
605+
/// Use this associated type for implementors that do not require any assumptions
606+
/// about the pointer type (apart from validity).
607+
/// The implementation will enable [`CassBorrowedPtr`] manipulation via [`RefFFI`]
608+
///
609+
/// C API user is not responsible for freeing associated memory manually. The memory
610+
/// should be freed automatically, when the owner is being dropped.
611+
///
612+
/// An example of such implementor would be [`CassRow`](crate::query_result::CassRow):
613+
/// - its lifetime is tied to the lifetime of CassResult
614+
/// - user only "borrows" the pointer - he is not responsible for freeing the memory
615+
pub struct FromRef;
616+
impl<T> origin_sealed::FromRefSealed for T where T: FFI<Origin = FromRef> {}
617+
impl<T> RefFFI for T where T: FFI<Origin = FromRef> {}
618+
545619
/// ```compile_fail,E0499
546620
/// # use scylla_cpp_driver::argconv::{CassOwnedMutPtr, CassBorrowedMutPtr, CMut};
547-
/// # use scylla_cpp_driver::argconv::BoxFFI;
621+
/// # use scylla_cpp_driver::argconv::{FFI, BoxFFI, FromBox};
548622
/// struct Foo;
549-
/// impl BoxFFI for Foo {}
623+
/// impl FFI for Foo {
624+
/// type Origin = FromBox;
625+
/// }
550626
///
551627
/// let mut ptr: CassOwnedMutPtr<Foo, CMut> = BoxFFI::into_ptr(Box::new(Foo));
552628
/// let borrowed_mut_ptr1: CassBorrowedMutPtr<Foo, CMut> = ptr.borrow_mut();
@@ -558,9 +634,11 @@ fn _test_box_ffi_cannot_have_two_mutable_references() {}
558634

559635
/// ```compile_fail,E0502
560636
/// # use scylla_cpp_driver::argconv::{CassOwnedMutPtr, CassBorrowedPtr, CassBorrowedMutPtr, CConst, CMut};
561-
/// # use scylla_cpp_driver::argconv::BoxFFI;
637+
/// # use scylla_cpp_driver::argconv::{FFI, BoxFFI, FromBox};
562638
/// struct Foo;
563-
/// impl BoxFFI for Foo {}
639+
/// impl FFI for Foo {
640+
/// type Origin = FromBox;
641+
/// }
564642
///
565643
/// let mut ptr: CassOwnedMutPtr<Foo, CMut> = BoxFFI::into_ptr(Box::new(Foo));
566644
/// let borrowed_mut_ptr: CassBorrowedMutPtr<Foo, CMut> = ptr.borrow_mut();
@@ -572,9 +650,11 @@ fn _test_box_ffi_cannot_have_mutable_and_immutable_references_at_the_same_time()
572650

573651
/// ```compile_fail,E0505
574652
/// # use scylla_cpp_driver::argconv::{CassOwnedMutPtr, CassBorrowedPtr, CMut};
575-
/// # use scylla_cpp_driver::argconv::BoxFFI;
653+
/// # use scylla_cpp_driver::argconv::{FFI, BoxFFI, FromBox};
576654
/// struct Foo;
577-
/// impl BoxFFI for Foo {}
655+
/// impl FFI for Foo {
656+
/// type Origin = FromBox;
657+
/// }
578658
///
579659
/// let ptr: CassOwnedMutPtr<Foo, CMut> = BoxFFI::into_ptr(Box::new(Foo));
580660
/// let borrowed_ptr: CassBorrowedPtr<Foo, CMut> = ptr.borrow();
@@ -585,10 +665,12 @@ fn _test_box_ffi_cannot_free_while_having_borrowed_pointer() {}
585665

586666
/// ```compile_fail,E0505
587667
/// # use scylla_cpp_driver::argconv::{CassOwnedPtr, CassBorrowedPtr, CConst};
588-
/// # use scylla_cpp_driver::argconv::ArcFFI;
668+
/// # use scylla_cpp_driver::argconv::{FFI, ArcFFI, FromArc};
589669
/// # use std::sync::Arc;
590670
/// struct Foo;
591-
/// impl ArcFFI for Foo {}
671+
/// impl FFI for Foo {
672+
/// type Origin = FromArc;
673+
/// }
592674
///
593675
/// let ptr: CassOwnedPtr<Foo, CConst> = ArcFFI::into_ptr(Arc::new(Foo));
594676
/// let borrowed_ptr: CassBorrowedPtr<Foo, CConst> = ptr.borrow();
@@ -599,10 +681,12 @@ fn _test_arc_ffi_cannot_clone_after_free() {}
599681

600682
/// ```compile_fail,E0505
601683
/// # use scylla_cpp_driver::argconv::{CassBorrowedPtr, CConst};
602-
/// # use scylla_cpp_driver::argconv::ArcFFI;
684+
/// # use scylla_cpp_driver::argconv::{FFI, ArcFFI, FromArc};
603685
/// # use std::sync::Arc;
604686
/// struct Foo;
605-
/// impl ArcFFI for Foo {}
687+
/// impl FFI for Foo {
688+
/// type Origin = FromArc;
689+
/// }
606690
///
607691
/// let arc = Arc::new(Foo);
608692
/// let borrowed_ptr: CassBorrowedPtr<Foo, CConst> = ArcFFI::as_ptr(&arc);

scylla-rust-wrapper/src/batch.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use crate::argconv::{ArcFFI, BoxFFI, CMut, CassBorrowedMutPtr, CassBorrowedPtr, CassOwnedMutPtr};
1+
use crate::argconv::{
2+
ArcFFI, BoxFFI, CMut, CassBorrowedMutPtr, CassBorrowedPtr, CassOwnedMutPtr, FromBox, FFI,
3+
};
24
use crate::cass_error::CassError;
35
use crate::cass_types::CassConsistency;
46
use crate::cass_types::{make_batch_type, CassBatchType};
@@ -19,7 +21,9 @@ pub struct CassBatch {
1921
pub(crate) exec_profile: Option<PerStatementExecProfile>,
2022
}
2123

22-
impl BoxFFI for CassBatch {}
24+
impl FFI for CassBatch {
25+
type Origin = FromBox;
26+
}
2327

2428
#[derive(Clone)]
2529
pub struct CassBatchState {

scylla-rust-wrapper/src/cass_types.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,9 @@ pub enum CassDataTypeInner {
145145
Custom(String),
146146
}
147147

148-
impl ArcFFI for CassDataType {}
148+
impl FFI for CassDataType {
149+
type Origin = FromArc;
150+
}
149151

150152
impl CassDataTypeInner {
151153
/// Checks for equality during typechecks.

scylla-rust-wrapper/src/cluster.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ impl CassCluster {
167167
}
168168
}
169169

170-
impl BoxFFI for CassCluster {}
170+
impl FFI for CassCluster {
171+
type Origin = FromBox;
172+
}
171173

172174
pub struct CassCustomPayload;
173175

scylla-rust-wrapper/src/collection.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ pub struct CassCollection {
3636
pub items: Vec<CassCqlValue>,
3737
}
3838

39-
impl BoxFFI for CassCollection {}
39+
impl FFI for CassCollection {
40+
type Origin = FromBox;
41+
}
4042

4143
impl CassCollection {
4244
fn typecheck_on_append(&self, value: &Option<CassCqlValue>) -> CassError {

scylla-rust-wrapper/src/exec_profile.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use scylla::statement::Consistency;
1515

1616
use crate::argconv::{
1717
ptr_to_cstr_n, strlen, ArcFFI, BoxFFI, CMut, CassBorrowedMutPtr, CassBorrowedPtr,
18-
CassOwnedMutPtr,
18+
CassOwnedMutPtr, FromBox, FFI,
1919
};
2020
use crate::batch::CassBatch;
2121
use crate::cass_error::CassError;
@@ -40,7 +40,9 @@ pub struct CassExecProfile {
4040
load_balancing_config: LoadBalancingConfig,
4141
}
4242

43-
impl BoxFFI for CassExecProfile {}
43+
impl FFI for CassExecProfile {
44+
type Origin = FromBox;
45+
}
4446

4547
impl CassExecProfile {
4648
fn new() -> Self {

scylla-rust-wrapper/src/future.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ pub struct CassFuture {
6060
wait_for_value: Condvar,
6161
}
6262

63-
impl ArcFFI for CassFuture {}
63+
impl FFI for CassFuture {
64+
type Origin = FromArc;
65+
}
6466

6567
/// An error that can appear during `cass_future_wait_timed`.
6668
enum FutureError {

scylla-rust-wrapper/src/iterator.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::argconv::{
22
write_str_to_c, ArcFFI, BoxFFI, CConst, CMut, CassBorrowedMutPtr, CassBorrowedPtr,
3-
CassOwnedMutPtr, RefFFI,
3+
CassOwnedMutPtr, FromBox, RefFFI, FFI,
44
};
55
use crate::cass_error::CassError;
66
use crate::cass_types::{CassDataType, CassValueType};
@@ -117,7 +117,9 @@ pub enum CassIterator<'result_or_schema> {
117117
ColumnsMeta(CassColumnsMetaIterator<'result_or_schema>),
118118
}
119119

120-
impl BoxFFI for CassIterator<'_> {}
120+
impl FFI for CassIterator<'_> {
121+
type Origin = FromBox;
122+
}
121123

122124
#[no_mangle]
123125
pub unsafe extern "C" fn cass_iterator_free(iterator: CassOwnedMutPtr<CassIterator, CMut>) {

scylla-rust-wrapper/src/logging.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use crate::argconv::{arr_to_cstr, ptr_to_cstr, str_to_arr, CConst, CassBorrowedPtr, RefFFI};
1+
use crate::argconv::{
2+
arr_to_cstr, ptr_to_cstr, str_to_arr, CConst, CassBorrowedPtr, FromRef, RefFFI, FFI,
3+
};
24
use crate::cass_log_types::{CassLogLevel, CassLogMessage};
35
use crate::types::size_t;
46
use crate::LOGGER;
@@ -14,7 +16,9 @@ use tracing_subscriber::layer::Context;
1416
use tracing_subscriber::prelude::*;
1517
use tracing_subscriber::Layer;
1618

17-
impl RefFFI for CassLogMessage {}
19+
impl FFI for CassLogMessage {
20+
type Origin = FromRef;
21+
}
1822

1923
pub type CassLogCallback = Option<
2024
unsafe extern "C" fn(message: CassBorrowedPtr<CassLogMessage, CConst>, data: *mut c_void),

scylla-rust-wrapper/src/metadata.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ pub struct CassSchemaMeta {
1313
pub keyspaces: HashMap<String, CassKeyspaceMeta>,
1414
}
1515

16-
impl BoxFFI for CassSchemaMeta {}
16+
impl FFI for CassSchemaMeta {
17+
type Origin = FromBox;
18+
}
1719

1820
pub struct CassKeyspaceMeta {
1921
pub name: String,
@@ -25,7 +27,9 @@ pub struct CassKeyspaceMeta {
2527
}
2628

2729
// Owned by CassSchemaMeta
28-
impl RefFFI for CassKeyspaceMeta {}
30+
impl FFI for CassKeyspaceMeta {
31+
type Origin = FromRef;
32+
}
2933

3034
pub struct CassTableMeta {
3135
pub name: String,
@@ -40,7 +44,9 @@ pub struct CassTableMeta {
4044
// Either:
4145
// - owned by CassMaterializedViewMeta - won't be given to user
4246
// - Owned by CassKeyspaceMeta (in Arc), referenced (Weak) by CassMaterializedViewMeta
43-
impl RefFFI for CassTableMeta {}
47+
impl FFI for CassTableMeta {
48+
type Origin = FromRef;
49+
}
4450

4551
pub struct CassMaterializedViewMeta {
4652
pub name: String,
@@ -49,7 +55,9 @@ pub struct CassMaterializedViewMeta {
4955
}
5056

5157
// Shared ownership by CassKeyspaceMeta and CassTableMeta
52-
impl RefFFI for CassMaterializedViewMeta {}
58+
impl FFI for CassMaterializedViewMeta {
59+
type Origin = FromRef;
60+
}
5361

5462
pub struct CassColumnMeta {
5563
pub name: String,
@@ -58,7 +66,9 @@ pub struct CassColumnMeta {
5866
}
5967

6068
// Owned by CassTableMeta
61-
impl RefFFI for CassColumnMeta {}
69+
impl FFI for CassColumnMeta {
70+
type Origin = FromRef;
71+
}
6272

6373
pub fn create_table_metadata(table_name: &str, table_metadata: &Table) -> CassTableMeta {
6474
let mut columns_metadata = HashMap::new();

scylla-rust-wrapper/src/prepared.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ impl CassPrepared {
7373
}
7474
}
7575

76-
impl ArcFFI for CassPrepared {}
76+
impl FFI for CassPrepared {
77+
type Origin = FromArc;
78+
}
7779

7880
#[no_mangle]
7981
pub unsafe extern "C" fn cass_prepared_free(prepared_raw: CassOwnedPtr<CassPrepared, CConst>) {

scylla-rust-wrapper/src/query_error.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ pub enum CassErrorResult {
1919
Deserialization(#[from] DeserializationError),
2020
}
2121

22-
impl ArcFFI for CassErrorResult {}
22+
impl FFI for CassErrorResult {
23+
type Origin = FromArc;
24+
}
2325

2426
impl From<Consistency> for CassConsistency {
2527
fn from(c: Consistency) -> CassConsistency {

0 commit comments

Comments
 (0)