From 0d01789de9ef6631dc8a41c9232fb532c64ae904 Mon Sep 17 00:00:00 2001 From: Dan Burkert Date: Sun, 21 Dec 2014 21:23:15 -0800 Subject: [PATCH] close_db and drop_db are unsafe --- src/database.rs | 2 +- src/environment.rs | 40 ++++++++++++--------- src/transaction.rs | 90 ++++++++++++++++++++++++++++++---------------- 3 files changed, 83 insertions(+), 49 deletions(-) diff --git a/src/database.rs b/src/database.rs index 37fbb34..c9f9208 100644 --- a/src/database.rs +++ b/src/database.rs @@ -9,7 +9,7 @@ use transaction::{RwTransaction, Transaction}; /// A handle to an individual database in an environment. /// /// A database handle denotes the name and parameters of a database in an environment. -#[deriving(Clone, Copy)] +#[deriving(Show, Clone, Copy, Eq, PartialEq)] pub struct Database { dbi: ffi::MDB_dbi, } diff --git a/src/environment.rs b/src/environment.rs index f82205f..1fb96a9 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -116,27 +116,21 @@ impl Environment { } } - /// Close a database handle. Normally unnecessary. + /// Closes the database handle. Normally unnecessary. /// /// Closing a database handle is not necessary, but lets `Transaction::open_database` reuse the /// handle value. Usually it's better to set a bigger `EnvironmentBuilder::set_max_dbs`, unless /// that value would be large. - pub fn close_db(&mut self, name: Option<&str>) -> LmdbResult<()> { - let db = try!(self.open_db(name)); - unsafe { ffi::mdb_dbi_close(self.env, db.dbi()) }; - Ok(()) - } - - pub fn clear_db(&mut self, name: Option<&str>) -> LmdbResult<()> { - let db = try!(self.open_db(name)); - let txn = try!(self.begin_rw_txn()); - unsafe { lmdb_result(ffi::mdb_drop(txn.txn(), db.dbi(), 0)) } - } - - pub fn drop_db(&mut self, name: Option<&str>) -> LmdbResult<()> { - let db = try!(self.open_db(name)); - let txn = try!(self.begin_rw_txn()); - unsafe { lmdb_result(ffi::mdb_drop(txn.txn(), db.dbi(), 1)) } + /// + /// ## Unsafety + /// + /// This call is not mutex protected. Databases should only be closed by a single thread, and + /// only if no other threads are going to reference the database handle or one of its cursors + /// any further. Do not close a handle if an existing transaction has modified its database. + /// Doing so can cause misbehavior from database corruption to errors like + /// `LmdbError::BadValSize` (since the DB name is gone). + pub unsafe fn close_db(&mut self, db: Database) { + ffi::mdb_dbi_close(self.env, db.dbi()); } } @@ -304,6 +298,18 @@ mod test { assert!(env.open_db(Some("testdb")).is_ok()) } + #[test] + fn test_close_database() { + let dir = io::TempDir::new("test").unwrap(); + let mut env = Environment::new().set_max_dbs(10) + .open(dir.path(), io::USER_RWX) + .unwrap(); + + let db = env.create_db(Some("db"), DatabaseFlags::empty()).unwrap(); + unsafe { env.close_db(db); } + assert!(env.open_db(Some("db")).is_ok()); + } + #[test] fn test_sync() { let dir = io::TempDir::new("test").unwrap(); diff --git a/src/transaction.rs b/src/transaction.rs index ba1cf37..902fd5f 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -308,6 +308,21 @@ impl <'env> RwTransaction<'env> { } } + /// Empties the given database. All items will be removed. + pub fn clear_db(&mut self, db: Database) -> LmdbResult<()> { + unsafe { lmdb_result(ffi::mdb_drop(self.txn(), db.dbi(), 0)) } + } + + /// Drops the database from the environment. + /// + /// ## Unsafety + /// + /// This method is unsafe in the same ways as `Environment::close_db`, and should be used + /// accordingly. + pub unsafe fn drop_db(&mut self, db: Database) -> LmdbResult<()> { + lmdb_result(ffi::mdb_drop(self.txn, db.dbi(), 1)) + } + /// Begins a new nested transaction inside of this transaction. pub fn begin_nested_txn<'txn>(&'txn mut self) -> LmdbResult> { let mut nested: *mut ffi::MDB_txn = ptr::null_mut(); @@ -390,37 +405,6 @@ mod test { assert_eq!(txn.get(db, b"key1"), Err(LmdbError::NotFound)); } - #[test] - fn test_close_database() { - let dir = io::TempDir::new("test").unwrap(); - let mut env = Environment::new().set_max_dbs(10) - .open(dir.path(), io::USER_RWX) - .unwrap(); - - env.create_db(Some("db"), DatabaseFlags::empty()).unwrap(); - env.close_db(Some("db")).unwrap(); - assert!(env.open_db(Some("db")).is_ok()); - } - - #[test] - fn test_clear_db() { - let dir = io::TempDir::new("test").unwrap(); - let mut env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); - - { - let db = env.open_db(None).unwrap(); - let mut txn = env.begin_rw_txn().unwrap(); - txn.put(db, b"key", b"val", WriteFlags::empty()).unwrap(); - txn.commit().unwrap(); - } - - env.clear_db(None).unwrap(); - - let db = env.open_db(None).unwrap(); - let txn = env.begin_ro_txn().unwrap(); - txn.get(db, b"key").is_err(); - } - #[test] fn test_inactive_txn() { let dir = io::TempDir::new("test").unwrap(); @@ -459,6 +443,50 @@ mod test { assert_eq!(txn.get(db, b"key2"), Err(LmdbError::NotFound)); } + #[test] + fn test_clear_db() { + let dir = io::TempDir::new("test").unwrap(); + let env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); + let db = env.open_db(None).unwrap(); + + { + let mut txn = env.begin_rw_txn().unwrap(); + txn.put(db, b"key", b"val", WriteFlags::empty()).unwrap(); + txn.commit().unwrap(); + } + + { + let mut txn = env.begin_rw_txn().unwrap(); + txn.clear_db(db).unwrap(); + txn.commit().unwrap(); + } + + let txn = env.begin_ro_txn().unwrap(); + assert_eq!(txn.get(db, b"key"), Err(LmdbError::NotFound)); + } + + + #[test] + fn test_drop_db() { + let dir = io::TempDir::new("test").unwrap(); + let env = Environment::new().set_max_dbs(2) + .open(dir.path(), io::USER_RWX).unwrap(); + let db = env.create_db(Some("test"), DatabaseFlags::empty()).unwrap(); + + { + let mut txn = env.begin_rw_txn().unwrap(); + txn.put(db, b"key", b"val", WriteFlags::empty()).unwrap(); + txn.commit().unwrap(); + } + { + let mut txn = env.begin_rw_txn().unwrap(); + unsafe { txn.drop_db(db).unwrap(); } + txn.commit().unwrap(); + } + + assert_eq!(env.open_db(Some("test")), Err(LmdbError::NotFound)); + } + #[test] fn test_concurrent_readers_single_writer() { let dir = io::TempDir::new("test").unwrap();