Fix memory leak issues around Collection, Scope & QueryIndex

Author: Nao-ris

src/collection.rs

@@ -63,11 +63,16 @@ impl Collection {
         }
     }
 
+    /// References the object without taking ownership and increasing it's reference counter
+    pub(crate) const fn wrap(cbl_ref: *mut CBLCollection) -> Self {
+        Self { cbl_ref }
+    }
+
     ////////
 
     /// Returns the scope of the collection.
     pub fn scope(&self) -> Scope {
-        unsafe { Scope::retain(CBLCollection_Scope(self.get_ref())) }
+        unsafe { Scope::wrap(CBLCollection_Scope(self.get_ref())) }
     }
 
     /// Returns the collection name.
@@ -90,7 +95,7 @@ impl Collection {
 
     /// Returns the collection's database.
     pub fn database(&self) -> Database {
-        unsafe { Database::wrap(CBLCollection_Database(self.get_ref())) }
+        unsafe { Database::retain(CBLCollection_Database(self.get_ref())) }
     }
 
     /// Returns the number of documents in the collection.

src/database.rs

@@ -418,7 +418,7 @@ impl Database {
             if scope.is_null() {
                 None
             } else {
-                Some(Scope::retain(scope))
+                Some(Scope::wrap(scope))
             }
         })
     }
@@ -445,7 +445,7 @@ impl Database {
             if collection.is_null() {
                 None
             } else {
-                Some(Collection::retain(collection))
+                Some(Collection::wrap(collection))
             }
         })
     }
@@ -474,7 +474,7 @@ impl Database {
             )
         };
 
-        check_error(&error).map(|()| Collection::retain(collection))
+        check_error(&error).map(|()| Collection::wrap(collection))
     }
 
     /// Delete an existing collection.
@@ -499,7 +499,7 @@ impl Database {
         let mut error = CBLError::default();
         let scope = unsafe { CBLDatabase_DefaultScope(self.get_ref(), &mut error) };
 
-        check_error(&error).map(|()| Scope::retain(scope))
+        check_error(&error).map(|()| Scope::wrap(scope))
     }
 
     /// Returns the default collection.
@@ -511,7 +511,7 @@ impl Database {
             if collection.is_null() {
                 None
             } else {
-                Some(Collection::retain(collection))
+                Some(Collection::wrap(collection))
             }
         })
     }
@@ -526,7 +526,7 @@ impl Database {
         if collection.is_null() {
             Err(Error::cbl_error(CouchbaseLiteError::NotFound))
         } else {
-            Ok(Collection::retain(collection))
+            Ok(Collection::wrap(collection))
         }
     }
 

src/index.rs

@@ -10,7 +10,7 @@ use crate::{
     slice::{from_str, from_c_str, Slice},
     QueryLanguage, Array,
     collection::Collection,
-    check_error, retain, CouchbaseLiteError,
+    check_error, release, retain, CouchbaseLiteError,
 };
 use std::ffi::CString;
 
@@ -149,12 +149,18 @@ impl QueryIndex {
     //////// CONSTRUCTORS:
 
     /// Takes ownership of the object and increase it's reference counter.
+    #[allow(dead_code)]
     pub(crate) fn retain(cbl_ref: *mut CBLQueryIndex) -> Self {
         Self {
             cbl_ref: unsafe { retain(cbl_ref) },
         }
     }
 
+    /// References the object without taking ownership and increasing it's reference counter
+    pub(crate) const fn wrap(cbl_ref: *mut CBLQueryIndex) -> Self {
+        Self { cbl_ref }
+    }
+
     ////////
 
     /// Returns the index's name.
@@ -172,6 +178,12 @@ impl QueryIndex {
     }
 }
 
+impl Drop for QueryIndex {
+    fn drop(&mut self) {
+        unsafe { release(self.get_ref()) }
+    }
+}
+
 impl Database {
     /// Creates a value index.
     /// Indexes are persistent.
@@ -280,7 +292,7 @@ impl Collection {
         let slice = from_str(name);
         let index = unsafe { CBLCollection_GetIndex(self.get_ref(), slice.get_ref(), &mut err) };
         if !err {
-            return Ok(QueryIndex::retain(index));
+            return Ok(QueryIndex::wrap(index));
         }
         failure(err)
     }

src/scope.rs

@@ -19,12 +19,20 @@ pub struct Scope {
 impl Scope {
     pub const DEFAULT_NAME: &str = "_default";
 
+    //////// CONSTRUCTORS:
+
+    /// Takes ownership of the object and increase it's reference counter.
     pub(crate) fn retain(cbl_ref: *mut CBLScope) -> Self {
         Self {
             cbl_ref: unsafe { retain(cbl_ref) },
         }
     }
 
+    /// References the object without taking ownership and increasing it's reference counter
+    pub(crate) const fn wrap(cbl_ref: *mut CBLScope) -> Self {
+        Self { cbl_ref }
+    }
+
     /** Returns the name of the scope. */
     pub fn name(&self) -> String {
         unsafe {
@@ -36,7 +44,7 @@ impl Scope {
 
     /** Returns the scope's database */
     pub fn database(&self) -> Database {
-        unsafe { Database::wrap(CBLScope_Database(self.get_ref())) }
+        unsafe { Database::retain(CBLScope_Database(self.get_ref())) }
     }
 
     /** Returns the names of all collections in the scope. */
@@ -63,7 +71,7 @@ impl Scope {
             if collection.is_null() {
                 None
             } else {
-                Some(Collection::retain(collection))
+                Some(Collection::wrap(collection))
             }
         })
     }

tests/utils.rs

@@ -44,18 +44,21 @@ where
     init_logging();
 
     let start_inst_count = instance_count();
-    let tmp_dir = TempDir::new("cbl_rust").expect("create temp dir");
-    let cfg = DatabaseConfiguration {
-        directory: tmp_dir.path(),
-        #[cfg(feature = "enterprise")]
-        encryption_key: None,
-    };
-    let mut db = Database::open(DB_NAME, Some(cfg)).expect("open db");
-    assert!(Database::exists(DB_NAME, tmp_dir.path()));
-
-    f(&mut db);
-
-    db.delete().unwrap();
+
+    {
+        let tmp_dir = TempDir::new("cbl_rust").expect("create temp dir");
+        let cfg = DatabaseConfiguration {
+            directory: tmp_dir.path(),
+            #[cfg(feature = "enterprise")]
+            encryption_key: None,
+        };
+        let mut db = Database::open(DB_NAME, Some(cfg)).expect("open db");
+        assert!(Database::exists(DB_NAME, tmp_dir.path()));
+
+        f(&mut db);
+
+        db.delete().unwrap();
+    }
 
     if LEAK_CHECK.is_some() {
         info!("Checking if Couchbase Lite objects were leaked by this test");