Fix panic when calling delete_table() for a table that is open

src/multimap_table.rs

@@ -7,7 +7,7 @@ use crate::tree_store::{
     RawBtree, RawLeafBuilder, TransactionalMemory, UntypedBtreeMut, BRANCH, LEAF, MAX_VALUE_LENGTH,
 };
 use crate::types::{RedbKey, RedbValue, TypeName};
-use crate::{AccessGuard, Result, StorageError, WriteTransaction};
+use crate::{AccessGuard, MultimapTableHandle, Result, StorageError, WriteTransaction};
 use std::borrow::Borrow;
 use std::cmp::max;
 use std::convert::TryInto;
@@ -720,6 +720,14 @@ pub struct MultimapTable<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static>
     _value_type: PhantomData<V>,
 }
 
+impl<K: RedbKey + 'static, V: RedbKey + 'static> MultimapTableHandle
+    for MultimapTable<'_, '_, K, V>
+{
+    fn name(&self) -> &str {
+        &self.name
+    }
+}
+
 impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, 'txn, K, V> {
     pub(crate) fn new(
         name: &str,
@@ -1120,7 +1128,7 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTabl
     }
 }
 
-impl<K: RedbKey, V: RedbKey> Sealed for MultimapTable<'_, '_, K, V> {}
+impl<K: RedbKey + 'static, V: RedbKey + 'static> Sealed for MultimapTable<'_, '_, K, V> {}
 
 impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> Drop
     for MultimapTable<'db, 'txn, K, V>

src/table.rs

@@ -4,8 +4,8 @@ use crate::tree_store::{
     PageHint, PageNumber, TransactionalMemory, MAX_VALUE_LENGTH,
 };
 use crate::types::{RedbKey, RedbValue, RedbValueMutInPlace};
-use crate::Result;
 use crate::{AccessGuard, StorageError, WriteTransaction};
+use crate::{Result, TableHandle};
 use std::borrow::Borrow;
 use std::fmt::{Debug, Formatter};
 use std::ops::RangeBounds;
@@ -62,6 +62,12 @@ pub struct Table<'db, 'txn, K: RedbKey + 'static, V: RedbValue + 'static> {
     tree: BtreeMut<'txn, K, V>,
 }
 
+impl<K: RedbKey + 'static, V: RedbValue + 'static> TableHandle for Table<'_, '_, K, V> {
+    fn name(&self) -> &str {
+        &self.name
+    }
+}
+
 impl<'db, 'txn, K: RedbKey + 'static, V: RedbValue + 'static> Table<'db, 'txn, K, V> {
     pub(crate) fn new(
         name: &str,

src/transactions.rs

@@ -360,6 +360,39 @@ impl<'db> TableNamespace<'db> {
         ))
     }
 
+    #[track_caller]
+    fn inner_delete(&mut self, name: &str, table_type: TableType) -> Result<bool, TableError> {
+        if let Some(location) = self.open_tables.get(name) {
+            return Err(TableError::TableAlreadyOpen(name.to_string(), location));
+        }
+
+        self.table_tree.delete_table(name, table_type)
+    }
+
+    #[track_caller]
+    fn delete_table<'txn>(
+        &mut self,
+        transaction: &'txn WriteTransaction<'db>,
+        name: &str,
+    ) -> Result<bool, TableError> {
+        #[cfg(feature = "logging")]
+        info!("Deleting table: {}", name);
+        transaction.dirty.store(true, Ordering::Release);
+        self.inner_delete(name, TableType::Normal)
+    }
+
+    #[track_caller]
+    fn delete_multimap_table<'txn>(
+        &mut self,
+        transaction: &'txn WriteTransaction<'db>,
+        name: &str,
+    ) -> Result<bool, TableError> {
+        #[cfg(feature = "logging")]
+        info!("Deleting multimap table: {}", name);
+        transaction.dirty.store(true, Ordering::Release);
+        self.inner_delete(name, TableType::Multimap)
+    }
+
     pub(crate) fn close_table<K: RedbKey + 'static, V: RedbValue + 'static>(
         &mut self,
         name: &str,
@@ -741,14 +774,10 @@ impl<'db> WriteTransaction<'db> {
     ///
     /// Returns a bool indicating whether the table existed
     pub fn delete_table(&self, definition: impl TableHandle) -> Result<bool, TableError> {
-        #[cfg(feature = "logging")]
-        info!("Deleting table: {}", definition.name());
-        self.dirty.store(true, Ordering::Release);
-        self.tables
-            .lock()
-            .unwrap()
-            .table_tree
-            .delete_table(definition.name(), TableType::Normal)
+        let name = definition.name().to_string();
+        // Drop the definition so that callers can pass in a `Table` or `MultimapTable` to delete, without getting a TableAlreadyOpen error
+        drop(definition);
+        self.tables.lock().unwrap().delete_table(self, &name)
     }
 
     /// Delete the given table
@@ -758,14 +787,13 @@ impl<'db> WriteTransaction<'db> {
         &self,
         definition: impl MultimapTableHandle,
     ) -> Result<bool, TableError> {
-        #[cfg(feature = "logging")]
-        info!("Deleting multimap table: {}", definition.name());
-        self.dirty.store(true, Ordering::Release);
+        let name = definition.name().to_string();
+        // Drop the definition so that callers can pass in a `Table` or `MultimapTable` to delete, without getting a TableAlreadyOpen error
+        drop(definition);
         self.tables
             .lock()
             .unwrap()
-            .table_tree
-            .delete_table(definition.name(), TableType::Multimap)
+            .delete_multimap_table(self, &name)
     }
 
     /// List all the tables

tests/basic_tests.rs

@@ -1,7 +1,7 @@
 use redb::backends::InMemoryBackend;
 use redb::{
     Database, MultimapTableDefinition, MultimapTableHandle, Range, ReadableTable, RedbKey,
-    RedbValue, TableDefinition, TableHandle, TypeName,
+    RedbValue, TableDefinition, TableError, TableHandle, TypeName,
 };
 use std::cmp::Ordering;
 #[cfg(not(target_os = "wasi"))]
@@ -812,6 +812,34 @@ fn delete() {
     assert_eq!(table.len().unwrap(), 1);
 }
 
+#[test]
+fn delete_open_table() {
+    let tmpfile = create_tempfile();
+    let db = Database::create(tmpfile.path()).unwrap();
+    let write_txn = db.begin_write().unwrap();
+    {
+        let table = write_txn.open_table(STR_TABLE).unwrap();
+        assert!(matches!(
+            write_txn.delete_table(STR_TABLE).unwrap_err(),
+            TableError::TableAlreadyOpen(_, _)
+        ));
+        drop(table);
+    }
+    write_txn.commit().unwrap();
+}
+
+#[test]
+fn delete_table() {
+    let tmpfile = create_tempfile();
+    let db = Database::create(tmpfile.path()).unwrap();
+    let write_txn = db.begin_write().unwrap();
+    {
+        let table = write_txn.open_table(STR_TABLE).unwrap();
+        assert!(write_txn.delete_table(table).unwrap());
+    }
+    write_txn.commit().unwrap();
+}
+
 #[test]
 fn no_dirty_reads() {
     let tmpfile = create_tempfile();