close_db and drop_db are unsafe

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,
 }

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));
+    /// ## Unsafety
-        unsafe { ffi::mdb_dbi_close(self.env, db.dbi()) };
+    ///
-        Ok(())
+    /// 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.
-    pub fn clear_db(&mut self, name: Option<&str>) -> LmdbResult<()> {
+    /// Doing so can cause misbehavior from database corruption to errors like
-        let db = try!(self.open_db(name));
+    /// `LmdbError::BadValSize` (since the DB name is gone).
-        let txn = try!(self.begin_rw_txn());
+    pub unsafe fn close_db(&mut self, db: Database) {
-        unsafe { lmdb_result(ffi::mdb_drop(txn.txn(), db.dbi(), 0)) }
+        ffi::mdb_dbi_close(self.env, db.dbi());
-    }
-
-    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)) }
     }
 }
 
@@ -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();

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<RwTransaction<'txn>> {
         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();