switch to keeping the vec in a box

Author: tantaman

core/rs/core/src/alter.rs

@@ -1,10 +1,10 @@
 // Not yet fully migrated from `crsqlite.c`
 
-use core::ffi::{c_char, c_int, CStr};
-
+use alloc::boxed::Box;
 use alloc::format;
 use alloc::string::String;
 use alloc::vec::Vec;
+use core::ffi::{c_char, c_int, CStr};
 use core::mem;
 #[cfg(not(feature = "std"))]
 use num_traits::FromPrimitive;
@@ -99,11 +99,8 @@ unsafe fn compact_post_alter(
             }
             return Err(ResultCode::ERROR);
         }
-        let table_infos = mem::ManuallyDrop::new(Vec::from_raw_parts(
-            (*ext_data).tableInfos as *mut TableInfo,
-            (*ext_data).tableInfosLen as usize,
-            (*ext_data).tableInfosCap as usize,
-        ));
+        let table_infos =
+            mem::ManuallyDrop::new(Box::from_raw((*ext_data).tableInfos as *mut Vec<TableInfo>));
         let table_info = table_infos.iter().find(|x| x.tbl_name == tbl_name_str);
         if table_info.is_none() {
             return Err(ResultCode::ERROR);

core/rs/core/src/c.rs

@@ -59,8 +59,6 @@ pub struct crsql_ExtData {
     pub siteId: *mut ::core::ffi::c_uchar,
     pub pDbVersionStmt: *mut sqlite::stmt,
     pub tableInfos: *mut ::core::ffi::c_void,
-    pub tableInfosLen: ::core::ffi::c_int,
-    pub tableInfosCap: ::core::ffi::c_int,
     pub rowsImpacted: ::core::ffi::c_int,
     pub seq: ::core::ffi::c_int,
     pub pSetSyncBitStmt: *mut sqlite::stmt,
@@ -109,6 +107,7 @@ extern "C" {
         db: *mut sqlite::sqlite3,
         siteIdBuffer: *mut c_char,
     ) -> *mut crsql_ExtData;
+    pub fn crsql_freeExtData(pExtData: *mut crsql_ExtData);
 }
 
 #[test]
@@ -263,7 +262,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     let ptr = UNINIT.as_ptr();
     assert_eq!(
         ::core::mem::size_of::<crsql_ExtData>(),
-        136usize,
+        128usize,
         concat!("Size of: ", stringify!(crsql_ExtData))
     );
     assert_eq!(
@@ -373,29 +372,9 @@ fn bindgen_test_layout_crsql_ExtData() {
             stringify!(tableInfos)
         )
     );
-    assert_eq!(
-        unsafe { ::core::ptr::addr_of!((*ptr).tableInfosLen) as usize - ptr as usize },
-        72usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(crsql_ExtData),
-            "::",
-            stringify!(tableInfosLen)
-        )
-    );
-    assert_eq!(
-        unsafe { ::core::ptr::addr_of!((*ptr).tableInfosCap) as usize - ptr as usize },
-        76usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(crsql_ExtData),
-            "::",
-            stringify!(tableInfosCap)
-        )
-    );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).rowsImpacted) as usize - ptr as usize },
-        80usize,
+        72usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -405,7 +384,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).seq) as usize - ptr as usize },
-        84usize,
+        76usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -415,7 +394,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).pSetSyncBitStmt) as usize - ptr as usize },
-        88usize,
+        80usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -425,7 +404,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).pClearSyncBitStmt) as usize - ptr as usize },
-        96usize,
+        88usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -435,7 +414,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).pSetSiteIdOrdinalStmt) as usize - ptr as usize },
-        104usize,
+        96usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -445,7 +424,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).pSelectSiteIdOrdinalStmt) as usize - ptr as usize },
-        112usize,
+        104usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -455,7 +434,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).pSelectClockTablesStmt) as usize - ptr as usize },
-        120usize,
+        112usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),
@@ -465,7 +444,7 @@ fn bindgen_test_layout_crsql_ExtData() {
     );
     assert_eq!(
         unsafe { ::core::ptr::addr_of!((*ptr).pStmtCache) as usize - ptr as usize },
-        128usize,
+        120usize,
         concat!(
             "Offset of field: ",
             stringify!(crsql_ExtData),

core/rs/core/src/changes_vtab.rs

@@ -5,6 +5,7 @@ use crate::stmt_cache::{
     get_cache_key, get_cached_stmt, reset_cached_stmt, set_cached_stmt, CachedStmtType,
 };
 use crate::tableinfo::{crsql_ensure_table_infos_are_up_to_date, TableInfo};
+use alloc::boxed::Box;
 use alloc::format;
 use alloc::string::String;
 use alloc::vec::Vec;
@@ -290,16 +291,14 @@ unsafe fn changes_filter(
     }
 
     // nothing to fetch, no crrs exist.
-    if (*(*tab).pExtData).tableInfosLen == 0 {
+    let tbl_infos = mem::ManuallyDrop::new(Box::from_raw(
+        (*(*tab).pExtData).tableInfos as *mut Vec<TableInfo>,
+    ));
+    if tbl_infos.len() == 0 {
         return Ok(ResultCode::OK);
     }
 
-    let table_infos = mem::ManuallyDrop::new(Vec::from_raw_parts(
-        (*(*tab).pExtData).tableInfos as *mut TableInfo,
-        (*(*tab).pExtData).tableInfosLen as usize,
-        (*(*tab).pExtData).tableInfosCap as usize,
-    ));
-    let sql = changes_union_query(&table_infos, idx_str)?;
+    let sql = changes_union_query(&tbl_infos, idx_str)?;
 
     let stmt = db.prepare_v2(&sql)?;
     for (i, arg) in args.iter().enumerate() {
@@ -375,10 +374,8 @@ unsafe fn changes_next(
         .column_int64(ClockUnionColumn::RowId as i32);
     (*cursor).dbVersion = db_version;
 
-    let tbl_infos = mem::ManuallyDrop::new(Vec::from_raw_parts(
-        (*(*(*cursor).pTab).pExtData).tableInfos as *mut TableInfo,
-        (*(*(*cursor).pTab).pExtData).tableInfosLen as usize,
-        (*(*(*cursor).pTab).pExtData).tableInfosCap as usize,
+    let tbl_infos = mem::ManuallyDrop::new(Box::from_raw(
+        (*(*(*cursor).pTab).pExtData).tableInfos as *mut Vec<TableInfo>,
     ));
     // TODO: will this work given `insert_tbl` is null termed?
     let tbl_info_index = tbl_infos.iter().position(|x| x.tbl_name == tbl);

core/rs/core/src/changes_vtab_write.rs

@@ -1,3 +1,4 @@
+use alloc::boxed::Box;
 use alloc::ffi::CString;
 use alloc::format;
 use alloc::string::String;
@@ -591,10 +592,8 @@ unsafe fn merge_insert(
     }
 
     let insert_site_id = insert_site_id.blob();
-    let tbl_infos = mem::ManuallyDrop::new(Vec::from_raw_parts(
-        (*(*tab).pExtData).tableInfos as *mut TableInfo,
-        (*(*tab).pExtData).tableInfosLen as usize,
-        (*(*tab).pExtData).tableInfosCap as usize,
+    let tbl_infos = mem::ManuallyDrop::new(Box::from_raw(
+        (*(*tab).pExtData).tableInfos as *mut Vec<TableInfo>,
     ));
     // TODO: will this work given `insert_tbl` is null termed?
     let tbl_info_index = tbl_infos.iter().position(|x| x.tbl_name == insert_tbl);

core/rs/core/src/tableinfo.rs

@@ -3,13 +3,16 @@ use crate::c::crsql_ExtData;
 use crate::c::crsql_fetchPragmaSchemaVersion;
 use crate::c::TABLE_INFO_SCHEMA_VERSION;
 use crate::util::Countable;
+use alloc::boxed::Box;
 use alloc::format;
 use alloc::string::String;
 use alloc::vec;
 use alloc::vec::Vec;
 use core::ffi::c_char;
 use core::ffi::c_int;
+use core::ffi::c_void;
 use core::mem;
+use core::mem::forget;
 use num_traits::ToPrimitive;
 use sqlite_nostd as sqlite;
 use sqlite_nostd::Connection;
@@ -36,12 +39,14 @@ pub struct ColumnInfo {
 
 #[no_mangle]
 pub extern "C" fn crsql_init_table_info_vec(ext_data: *mut crsql_ExtData) {
-    let vec = vec![];
-    let (ptr, len, cap) = vec.into_raw_parts();
+    let vec: Vec<TableInfo> = vec![];
+    unsafe { (*ext_data).tableInfos = Box::into_raw(Box::new(vec)) as *mut c_void }
+}
+
+#[no_mangle]
+pub extern "C" fn crsql_drop_table_info_vec(ext_data: *mut crsql_ExtData) {
     unsafe {
-        (*ext_data).tableInfos = ptr;
-        (*ext_data).tableInfosLen = len as i32;
-        (*ext_data).tableInfosCap = cap as i32;
+        drop(Box::from_raw((*ext_data).tableInfos as *mut Vec<TableInfo>));
     }
 }
 
@@ -58,27 +63,23 @@ pub extern "C" fn crsql_ensure_table_infos_are_up_to_date(
         return ResultCode::ERROR as c_int;
     }
 
-    let mut table_infos = unsafe {
-        mem::ManuallyDrop::new(Vec::from_raw_parts(
-            (*ext_data).tableInfos as *mut TableInfo,
-            (*ext_data).tableInfosLen as usize,
-            (*ext_data).tableInfosCap as usize,
-        ))
-    };
+    let mut table_infos = unsafe { Box::from_raw((*ext_data).tableInfos as *mut Vec<TableInfo>) };
 
     if schema_changed > 0 || table_infos.len() == 0 {
-        table_infos.clear();
         match pull_all_table_infos(db, ext_data, err) {
             Ok(new_table_infos) => {
-                table_infos.extend(new_table_infos.into_iter());
+                *table_infos = new_table_infos;
+                forget(table_infos);
                 return ResultCode::OK as c_int;
             }
             Err(e) => {
+                forget(table_infos);
                 return e as c_int;
             }
         }
     }
 
+    forget(table_infos);
     return ResultCode::OK as c_int;
 }
 
@@ -107,7 +108,11 @@ fn pull_all_table_infos(
 
     let mut ret = vec![];
     for name in clock_table_names {
-        ret.push(pull_table_info(db, &name, err)?)
+        ret.push(pull_table_info(
+            db,
+            &name[0..(name.len() - "__crsql_clock".len())],
+            err,
+        )?)
     }
 
     Ok(ret)

core/rs/integration-check/tests/tableinfo.rs

@@ -1,6 +1,9 @@
-use std::ffi::{c_char, CString};
+use std::{
+    ffi::{c_char, CString},
+    mem,
+};
 
-use crsql_bundle::crsql_core;
+use crsql_bundle::crsql_core::{self, tableinfo::TableInfo};
 use sqlite::Connection;
 use sqlite_nostd as sqlite;
 
@@ -41,13 +44,32 @@ fn test_ensure_table_infos_are_up_to_date() {
     )
     .expect("made foo clock");
 
+    let stmt = c
+        .prepare_v2("SELECT tbl_name FROM sqlite_master WHERE type='table' AND tbl_name LIKE '%__crsql_clock'")
+        .expect("preped");
+    let r = stmt.step().expect("stepped");
+    assert_eq!(r, sqlite::ResultCode::ROW);
+
     let ext_data = unsafe { crsql_core::c::crsql_newExtData(raw_db, make_site()) };
     crsql_core::tableinfo::crsql_ensure_table_infos_are_up_to_date(
         raw_db,
         ext_data,
         make_err_ptr(),
     );
 
+    let table_infos = unsafe {
+        mem::ManuallyDrop::new(Box::from_raw((*ext_data).tableInfos as *mut Vec<TableInfo>))
+    };
+
+    assert_eq!(table_infos.len(), 1);
+    assert_eq!(table_infos[0].tbl_name, "foo");
+
+    unsafe {
+        crsql_core::c::crsql_freeExtData(ext_data);
+    };
+
+    // free ext_data since it has prepared statements
+
     // ideally we can check if it does a repull or not...
     // we could do this by mutating table infos to something unexpected and checking it is still that.
     decrement_counter();

core/src/ext-data.c

@@ -7,6 +7,7 @@
 void crsql_init_stmt_cache(crsql_ExtData *pExtData);
 void crsql_clear_stmt_cache(crsql_ExtData *pExtData);
 void crsql_init_table_info_vec(crsql_ExtData *pExtData);
+void crsql_drop_table_info_vec(crsql_ExtData *pExtData);
 
 crsql_ExtData *crsql_newExtData(sqlite3 *db, unsigned char *siteIdBuffer) {
   crsql_ExtData *pExtData = sqlite3_malloc(sizeof *pExtData);
@@ -50,8 +51,6 @@ crsql_ExtData *crsql_newExtData(sqlite3 *db, unsigned char *siteIdBuffer) {
   pExtData->siteId = siteIdBuffer;
   pExtData->pDbVersionStmt = 0;
   pExtData->tableInfos = 0;
-  pExtData->tableInfosCap = 0;
-  pExtData->tableInfosLen = 0;
   pExtData->rowsImpacted = 0;
   pExtData->pStmtCache = 0;
   crsql_init_stmt_cache(pExtData);
@@ -77,6 +76,7 @@ void crsql_freeExtData(crsql_ExtData *pExtData) {
   sqlite3_finalize(pExtData->pSelectSiteIdOrdinalStmt);
   sqlite3_finalize(pExtData->pSelectClockTablesStmt);
   crsql_clear_stmt_cache(pExtData);
+  crsql_drop_table_info_vec(pExtData);
   sqlite3_free(pExtData);
 }
 

core/src/ext-data.h

@@ -29,8 +29,6 @@ struct crsql_ExtData {
   unsigned char *siteId;
   sqlite3_stmt *pDbVersionStmt;
   void *tableInfos;
-  int tableInfosLen;
-  int tableInfosCap;
 
   // tracks the number of rows impacted by all inserts into crsql_changes in the
   // current transaction. This number is reset on transaction commit.

core/src/ext-data.test.c

@@ -35,8 +35,6 @@ static void textNewExtData() {
   assert(pExtData->pDbVersionStmt == 0);
   // table info allocated to an empty vec
   assert(pExtData->tableInfos != 0);
-  assert(pExtData->tableInfosLen == 0);
-  assert(pExtData->tableInfosCap == 0);
 
   // data version should have been fetched
   assert(pExtData->pragmaDataVersion != -1);