Merge pull request #12 from spacejam/tyler_fix_memory_leaks

Fix a few memory leaks.  Add valgrind feature build.

Cargo.toml

@@ -5,3 +5,7 @@ description = "A Rust wrapper for Facebook's RocksDB embeddable database."
 version = "0.0.5"
 authors = ["Tyler Neely <t@jujit.su>"]
 license = "Apache-2.0"
+
+[features]
+default=[]
+valgrind=[]

README.md

@@ -45,9 +45,9 @@ use rocksdb::{RocksDBOptions, RocksDB, MergeOperands};
 fn concat_merge(new_key: &[u8], existing_val: Option<&[u8]>,
     operands: &mut MergeOperands) -> Vec<u8> {
     let mut result: Vec<u8> = Vec::with_capacity(operands.size_hint().0);
-    existing_val.map(|v| { result.push_all(v) });
+    existing_val.map(|v| { result.extend(v) });
     for op in operands {
-        result.push_all(op);
+        result.extend(op);
     }
     result
 }

src/ffi.rs

@@ -180,12 +180,14 @@ extern {
                         err: *mut i8
                         ) -> RocksDBInstance;
     pub fn rocksdb_writeoptions_create() -> RocksDBWriteOptions;
+    pub fn rocksdb_writeoptions_destroy(writeopts: RocksDBWriteOptions);
     pub fn rocksdb_put(db: RocksDBInstance,
                        writeopts: RocksDBWriteOptions,
                        k: *const u8, kLen: size_t,
                        v: *const u8, vLen: size_t,
                        err: *mut i8);
     pub fn rocksdb_readoptions_create() -> RocksDBReadOptions;
+    pub fn rocksdb_readoptions_destroy(readopts: RocksDBReadOptions);
     pub fn rocksdb_readoptions_set_snapshot(read_opts: RocksDBReadOptions,
                                             snapshot: RocksDBSnapshot);
     pub fn rocksdb_get(db: RocksDBInstance,
@@ -338,22 +340,22 @@ fn internal() {
         assert!(err.is_null());
 
         let writeopts = rocksdb_writeoptions_create();
-        let RocksDBWriteOptions(write_opt_ptr) = writeopts;
-        assert!(!write_opt_ptr.is_null());
+        assert!(!writeopts.0.is_null());
 
         let key = b"name\x00";
         let val = b"spacejam\x00";
 
-        rocksdb_put(db, writeopts, key.as_ptr(), 4, val.as_ptr(), 8, err);
+        rocksdb_put(db, writeopts.clone(), key.as_ptr(), 4, val.as_ptr(), 8, err);
+        rocksdb_writeoptions_destroy(writeopts);
         assert!(err.is_null());
 
         let readopts = rocksdb_readoptions_create();
-        let RocksDBReadOptions(read_opts_ptr) = readopts;
-        assert!(!read_opts_ptr.is_null());
+        assert!(!readopts.0.is_null());
 
         let val_len: size_t = 0;
         let val_len_ptr = &val_len as *const size_t;
-        rocksdb_get(db, readopts, key.as_ptr(), 4, val_len_ptr, err);
+        rocksdb_get(db, readopts.clone(), key.as_ptr(), 4, val_len_ptr, err);
+        rocksdb_readoptions_destroy(readopts);
         assert!(err.is_null());
         rocksdb_close(db);
         rocksdb_destroy_db(opts, cpath_ptr, err);

src/lib.rs

@@ -19,7 +19,6 @@
 #![feature(unique)]
 #![feature(path_ext)]
 #![feature(raw)]
-#![feature(vec_push_all)]
 
 pub use ffi as rocksdb_ffi;
 pub use ffi::{

src/main.rs

@@ -14,13 +14,13 @@
    limitations under the License.
 */
 #![feature(test)]
-#![feature(vec_push_all)]
 
 extern crate rocksdb;
 extern crate test;
 use rocksdb::{RocksDBOptions, RocksDB, MergeOperands, new_bloom_filter, Writable};
 use rocksdb::RocksDBCompactionStyle::RocksDBUniversalCompaction;
 
+#[cfg(not(feature = "valgrind"))]
 fn main() {
     let path = "/tmp/rust-rocksdb";
     let db = RocksDB::open_default(path).unwrap();
@@ -46,11 +46,11 @@ fn concat_merge(new_key: &[u8], existing_val: Option<&[u8]>,
     mut operands: &mut MergeOperands) -> Vec<u8> {
     let mut result: Vec<u8> = Vec::with_capacity(operands.size_hint().0);
     match existing_val {
-        Some(v) => result.push_all(v),
+        Some(v) => result.extend(v),
         None => (),
     }
     for op in operands {
-        result.push_all(op);
+        result.extend(op);
     }
     result
 }
@@ -82,6 +82,32 @@ fn custom_merge() {
     RocksDB::destroy(opts, path).is_ok();
 }
 
+#[cfg(feature = "valgrind")]
+fn main() {
+    let path = "_rust_rocksdb_valgrind";
+    let opts = RocksDBOptions::new();
+    opts.create_if_missing(true);
+    opts.add_merge_operator("test operator", concat_merge);
+    let db = RocksDB::open(opts, path).unwrap();
+    loop {
+        db.put(b"k1", b"a");
+        db.merge(b"k1", b"b");
+        db.merge(b"k1", b"c");
+        db.merge(b"k1", b"d");
+        db.merge(b"k1", b"efg");
+        db.merge(b"k1", b"h");
+        db.get(b"k1").map( |value| {
+            match value.to_utf8() {
+                Some(v) => (),
+                None => panic!("value corrupted"),
+            }
+        })
+            .on_absent( || { panic!("value not found") })
+            .on_error( |e| { panic!("error retrieving value: {}", e) });
+        db.delete(b"k1");
+    }
+}
+
 
 #[cfg(test)]
 mod tests  {

src/merge_operator.rs

@@ -153,11 +153,11 @@ fn test_provided_merge(new_key: &[u8], existing_val: Option<&[u8]>,
     let nops = operands.size_hint().0;
     let mut result: Vec<u8> = Vec::with_capacity(nops);
     match existing_val {
-        Some(v) => result.push_all(v),
+        Some(v) => result.extend(v),
         None => (),
     }
     for op in operands {
-        result.push_all(op);
+        result.extend(op);
     }
     result
 }

src/rocksdb.rs

@@ -132,7 +132,8 @@ impl RocksDB {
         unsafe {
             let writeopts = rocksdb_ffi::rocksdb_writeoptions_create();
             let err = 0 as *mut i8;
-            rocksdb_ffi::rocksdb_write(self.inner, writeopts, batch.inner, err);
+            rocksdb_ffi::rocksdb_write(self.inner, writeopts.clone(), batch.inner, err);
+            rocksdb_ffi::rocksdb_writeoptions_destroy(writeopts);
             if !err.is_null() {
                 return Err(error_message(err));
             }
@@ -153,8 +154,9 @@ impl RocksDB {
             let val_len: size_t = 0;
             let val_len_ptr = &val_len as *const size_t;
             let err = 0 as *mut i8;
-            let val = rocksdb_ffi::rocksdb_get(self.inner, readopts,
+            let val = rocksdb_ffi::rocksdb_get(self.inner, readopts.clone(),
                 key.as_ptr(), key.len() as size_t, val_len_ptr, err) as *mut u8;
+            rocksdb_ffi::rocksdb_readoptions_destroy(readopts);
             if !err.is_null() {
                 return RocksDBResult::Error(error_message(err));
             }
@@ -177,9 +179,10 @@ impl Writable for RocksDB {
         unsafe {
             let writeopts = rocksdb_ffi::rocksdb_writeoptions_create();
             let err = 0 as *mut i8;
-            rocksdb_ffi::rocksdb_put(self.inner, writeopts, key.as_ptr(),
+            rocksdb_ffi::rocksdb_put(self.inner, writeopts.clone(), key.as_ptr(),
                         key.len() as size_t, value.as_ptr(),
                         value.len() as size_t, err);
+            rocksdb_ffi::rocksdb_writeoptions_destroy(writeopts);
             if !err.is_null() {
                 return Err(error_message(err));
             }
@@ -191,9 +194,10 @@ impl Writable for RocksDB {
         unsafe {
             let writeopts = rocksdb_ffi::rocksdb_writeoptions_create();
             let err = 0 as *mut i8;
-            rocksdb_ffi::rocksdb_merge(self.inner, writeopts, key.as_ptr(),
+            rocksdb_ffi::rocksdb_merge(self.inner, writeopts.clone(), key.as_ptr(),
                         key.len() as size_t, value.as_ptr(),
                         value.len() as size_t, err);
+            rocksdb_ffi::rocksdb_writeoptions_destroy(writeopts);
             if !err.is_null() {
                 return Err(error_message(err));
             }
@@ -205,8 +209,9 @@ impl Writable for RocksDB {
         unsafe {
             let writeopts = rocksdb_ffi::rocksdb_writeoptions_create();
             let err = 0 as *mut i8;
-            rocksdb_ffi::rocksdb_delete(self.inner, writeopts, key.as_ptr(),
+            rocksdb_ffi::rocksdb_delete(self.inner, writeopts.clone(), key.as_ptr(),
                         key.len() as size_t, err);
+            rocksdb_ffi::rocksdb_writeoptions_destroy(writeopts);
             if !err.is_null() {
                 return Err(error_message(err));
             }