From dce2f225202a8384e4deb01be88ee88758499ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Wed, 18 Dec 2024 20:55:38 +0100 Subject: [PATCH 1/3] iter: hold reference to CassResult in CassResultIterator cpp-driver does not increase the reference count. The lifetime of the iterator is bound to the lifetime of result. --- scylla-rust-wrapper/src/query_result.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scylla-rust-wrapper/src/query_result.rs b/scylla-rust-wrapper/src/query_result.rs index 289a0a68..0b150ab8 100644 --- a/scylla-rust-wrapper/src/query_result.rs +++ b/scylla-rust-wrapper/src/query_result.rs @@ -278,7 +278,7 @@ fn get_column_value(column: CqlValue, column_type: &Arc) -> Value } pub struct CassResultIterator { - result: Arc, + result: &'static CassResult, position: Option, } @@ -824,7 +824,7 @@ pub unsafe extern "C" fn cass_iterator_get_materialized_view_meta( #[no_mangle] pub unsafe extern "C" fn cass_iterator_from_result(result: *const CassResult) -> *mut CassIterator { - let result_from_raw = ArcFFI::cloned_from_ptr(result); + let result_from_raw = ArcFFI::as_ref(result); let iterator = CassResultIterator { result: result_from_raw, From e761dccf0c0307be323db360df230ec8bcd7f8ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Wed, 18 Dec 2024 21:01:18 +0100 Subject: [PATCH 2/3] iter: parameterize CassIterator with lifetime It's more readable (and safe) to have an explicit lifetime instead of lifetime-erased references. --- scylla-rust-wrapper/src/query_result.rs | 114 +++++++++++++----------- 1 file changed, 61 insertions(+), 53 deletions(-) diff --git a/scylla-rust-wrapper/src/query_result.rs b/scylla-rust-wrapper/src/query_result.rs index 0b150ab8..66c00b0d 100644 --- a/scylla-rust-wrapper/src/query_result.rs +++ b/scylla-rust-wrapper/src/query_result.rs @@ -277,73 +277,73 @@ fn get_column_value(column: CqlValue, column_type: &Arc) -> Value } } -pub struct CassResultIterator { - result: &'static CassResult, +pub struct CassResultIterator<'result> { + result: &'result CassResult, position: Option, } -pub struct CassRowIterator { - row: &'static CassRow, +pub struct CassRowIterator<'result> { + row: &'result CassRow, position: Option, } -pub struct CassCollectionIterator { - value: &'static CassValue, +pub struct CassCollectionIterator<'result> { + value: &'result CassValue, count: u64, position: Option, } -pub struct CassMapIterator { - value: &'static CassValue, +pub struct CassMapIterator<'result> { + value: &'result CassValue, count: u64, position: Option, } -pub struct CassUdtIterator { - value: &'static CassValue, +pub struct CassUdtIterator<'result> { + value: &'result CassValue, count: u64, position: Option, } -pub struct CassSchemaMetaIterator { - value: &'static CassSchemaMeta, +pub struct CassSchemaMetaIterator<'schema> { + value: &'schema CassSchemaMeta, count: usize, position: Option, } -pub struct CassKeyspaceMetaIterator { - value: &'static CassKeyspaceMeta, +pub struct CassKeyspaceMetaIterator<'schema> { + value: &'schema CassKeyspaceMeta, count: usize, position: Option, } -pub struct CassTableMetaIterator { - value: &'static CassTableMeta, +pub struct CassTableMetaIterator<'schema> { + value: &'schema CassTableMeta, count: usize, position: Option, } -pub struct CassViewMetaIterator { - value: &'static CassMaterializedViewMeta, +pub struct CassViewMetaIterator<'schema> { + value: &'schema CassMaterializedViewMeta, count: usize, position: Option, } -pub enum CassIterator { - CassResultIterator(CassResultIterator), - CassRowIterator(CassRowIterator), - CassCollectionIterator(CassCollectionIterator), - CassMapIterator(CassMapIterator), - CassUdtIterator(CassUdtIterator), - CassSchemaMetaIterator(CassSchemaMetaIterator), - CassKeyspaceMetaTableIterator(CassKeyspaceMetaIterator), - CassKeyspaceMetaUserTypeIterator(CassKeyspaceMetaIterator), - CassKeyspaceMetaViewIterator(CassKeyspaceMetaIterator), - CassTableMetaIterator(CassTableMetaIterator), - CassViewMetaIterator(CassViewMetaIterator), +pub enum CassIterator<'result_or_schema> { + CassResultIterator(CassResultIterator<'result_or_schema>), + CassRowIterator(CassRowIterator<'result_or_schema>), + CassCollectionIterator(CassCollectionIterator<'result_or_schema>), + CassMapIterator(CassMapIterator<'result_or_schema>), + CassUdtIterator(CassUdtIterator<'result_or_schema>), + CassSchemaMetaIterator(CassSchemaMetaIterator<'result_or_schema>), + CassKeyspaceMetaTableIterator(CassKeyspaceMetaIterator<'result_or_schema>), + CassKeyspaceMetaUserTypeIterator(CassKeyspaceMetaIterator<'result_or_schema>), + CassKeyspaceMetaViewIterator(CassKeyspaceMetaIterator<'result_or_schema>), + CassTableMetaIterator(CassTableMetaIterator<'result_or_schema>), + CassViewMetaIterator(CassViewMetaIterator<'result_or_schema>), } -impl BoxFFI for CassIterator {} +impl BoxFFI for CassIterator<'_> {} #[no_mangle] pub unsafe extern "C" fn cass_iterator_free(iterator: *mut CassIterator) { @@ -823,7 +823,9 @@ pub unsafe extern "C" fn cass_iterator_get_materialized_view_meta( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_from_result(result: *const CassResult) -> *mut CassIterator { +pub unsafe extern "C" fn cass_iterator_from_result<'result>( + result: *const CassResult, +) -> *mut CassIterator<'result> { let result_from_raw = ArcFFI::as_ref(result); let iterator = CassResultIterator { @@ -835,7 +837,9 @@ pub unsafe extern "C" fn cass_iterator_from_result(result: *const CassResult) -> } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_from_row(row: *const CassRow) -> *mut CassIterator { +pub unsafe extern "C" fn cass_iterator_from_row<'result>( + row: *const CassRow, +) -> *mut CassIterator<'result> { let row_from_raw = RefFFI::as_ref(row); let iterator = CassRowIterator { @@ -847,9 +851,9 @@ pub unsafe extern "C" fn cass_iterator_from_row(row: *const CassRow) -> *mut Cas } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_from_collection( +pub unsafe extern "C" fn cass_iterator_from_collection<'result>( value: *const CassValue, -) -> *mut CassIterator { +) -> *mut CassIterator<'result> { let is_collection = cass_value_is_collection(value) != 0; if value.is_null() || !is_collection { @@ -873,7 +877,9 @@ pub unsafe extern "C" fn cass_iterator_from_collection( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_from_tuple(value: *const CassValue) -> *mut CassIterator { +pub unsafe extern "C" fn cass_iterator_from_tuple<'result>( + value: *const CassValue, +) -> *mut CassIterator<'result> { let tuple = RefFFI::as_ref(value); if let Some(Value::CollectionValue(Collection::Tuple(val))) = &tuple.value { @@ -891,7 +897,9 @@ pub unsafe extern "C" fn cass_iterator_from_tuple(value: *const CassValue) -> *m } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_from_map(value: *const CassValue) -> *mut CassIterator { +pub unsafe extern "C" fn cass_iterator_from_map<'result>( + value: *const CassValue, +) -> *mut CassIterator<'result> { let map = RefFFI::as_ref(value); if let Some(Value::CollectionValue(Collection::Map(val))) = &map.value { @@ -909,9 +917,9 @@ pub unsafe extern "C" fn cass_iterator_from_map(value: *const CassValue) -> *mut } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_fields_from_user_type( +pub unsafe extern "C" fn cass_iterator_fields_from_user_type<'result>( value: *const CassValue, -) -> *mut CassIterator { +) -> *mut CassIterator<'result> { let udt = RefFFI::as_ref(value); if let Some(Value::CollectionValue(Collection::UserDefinedType { fields, .. })) = &udt.value { @@ -929,9 +937,9 @@ pub unsafe extern "C" fn cass_iterator_fields_from_user_type( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_keyspaces_from_schema_meta( +pub unsafe extern "C" fn cass_iterator_keyspaces_from_schema_meta<'schema>( schema_meta: *const CassSchemaMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = BoxFFI::as_ref(schema_meta); let iterator = CassSchemaMetaIterator { @@ -944,9 +952,9 @@ pub unsafe extern "C" fn cass_iterator_keyspaces_from_schema_meta( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_tables_from_keyspace_meta( +pub unsafe extern "C" fn cass_iterator_tables_from_keyspace_meta<'schema>( keyspace_meta: *const CassKeyspaceMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = RefFFI::as_ref(keyspace_meta); let iterator = CassKeyspaceMetaIterator { @@ -961,9 +969,9 @@ pub unsafe extern "C" fn cass_iterator_tables_from_keyspace_meta( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_materialized_views_from_keyspace_meta( +pub unsafe extern "C" fn cass_iterator_materialized_views_from_keyspace_meta<'schema>( keyspace_meta: *const CassKeyspaceMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = RefFFI::as_ref(keyspace_meta); let iterator = CassKeyspaceMetaIterator { @@ -978,9 +986,9 @@ pub unsafe extern "C" fn cass_iterator_materialized_views_from_keyspace_meta( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_user_types_from_keyspace_meta( +pub unsafe extern "C" fn cass_iterator_user_types_from_keyspace_meta<'schema>( keyspace_meta: *const CassKeyspaceMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = RefFFI::as_ref(keyspace_meta); let iterator = CassKeyspaceMetaIterator { @@ -995,9 +1003,9 @@ pub unsafe extern "C" fn cass_iterator_user_types_from_keyspace_meta( } #[no_mangle] -pub unsafe extern "C" fn cass_iterator_columns_from_table_meta( +pub unsafe extern "C" fn cass_iterator_columns_from_table_meta<'schema>( table_meta: *const CassTableMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = RefFFI::as_ref(table_meta); let iterator = CassTableMetaIterator { @@ -1009,9 +1017,9 @@ pub unsafe extern "C" fn cass_iterator_columns_from_table_meta( BoxFFI::into_ptr(Box::new(CassIterator::CassTableMetaIterator(iterator))) } -pub unsafe extern "C" fn cass_iterator_materialized_views_from_table_meta( +pub unsafe extern "C" fn cass_iterator_materialized_views_from_table_meta<'schema>( table_meta: *const CassTableMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = RefFFI::as_ref(table_meta); let iterator = CassTableMetaIterator { @@ -1023,9 +1031,9 @@ pub unsafe extern "C" fn cass_iterator_materialized_views_from_table_meta( BoxFFI::into_ptr(Box::new(CassIterator::CassTableMetaIterator(iterator))) } -pub unsafe extern "C" fn cass_iterator_columns_from_materialized_view_meta( +pub unsafe extern "C" fn cass_iterator_columns_from_materialized_view_meta<'schema>( view_meta: *const CassMaterializedViewMeta, -) -> *mut CassIterator { +) -> *mut CassIterator<'schema> { let metadata = RefFFI::as_ref(view_meta); let iterator = CassViewMetaIterator { From 0a00d39845c756ee79ee04fab0895c644e90b101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Tue, 11 Mar 2025 13:35:54 +0100 Subject: [PATCH 3/3] future: change .clone() to Arc::clone() It's more readable and easier to spot where Arc cloning appears. --- scylla-rust-wrapper/src/future.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scylla-rust-wrapper/src/future.rs b/scylla-rust-wrapper/src/future.rs index a3c46f52..df2998dc 100644 --- a/scylla-rust-wrapper/src/future.rs +++ b/scylla-rust-wrapper/src/future.rs @@ -86,7 +86,7 @@ impl CassFuture { state: Mutex::new(Default::default()), wait_for_value: Condvar::new(), }); - let cass_fut_clone = cass_fut.clone(); + let cass_fut_clone = Arc::clone(&cass_fut); let join_handle = RUNTIME.spawn(async move { let r = fut.await; let maybe_cb = { @@ -360,7 +360,7 @@ pub unsafe extern "C" fn cass_future_get_result( ArcFFI::as_ref(future_raw) .with_waited_result(|r: &mut CassFutureResult| -> Option> { match r.as_ref().ok()? { - CassResultValue::QueryResult(qr) => Some(qr.clone()), + CassResultValue::QueryResult(qr) => Some(Arc::clone(qr)), _ => None, } }) @@ -374,7 +374,7 @@ pub unsafe extern "C" fn cass_future_get_error_result( ArcFFI::as_ref(future_raw) .with_waited_result(|r: &mut CassFutureResult| -> Option> { match r.as_ref().ok()? { - CassResultValue::QueryError(qr) => Some(qr.clone()), + CassResultValue::QueryError(qr) => Some(Arc::clone(qr)), _ => None, } }) @@ -388,7 +388,7 @@ pub unsafe extern "C" fn cass_future_get_prepared( ArcFFI::as_ref(future_raw) .with_waited_result(|r: &mut CassFutureResult| -> Option> { match r.as_ref().ok()? { - CassResultValue::Prepared(p) => Some(p.clone()), + CassResultValue::Prepared(p) => Some(Arc::clone(p)), _ => None, } })