diff --git a/src/tree_store/table_tree.rs b/src/tree_store/table_tree.rs index b4c6cb1d..53de8090 100644 --- a/src/tree_store/table_tree.rs +++ b/src/tree_store/table_tree.rs @@ -796,17 +796,65 @@ impl<'txn> TableTreeMut<'txn> { table_type: TableType, ) -> Result { if let Some(definition) = self.get_table_untyped(name, table_type)? { - if let Some(header) = definition.get_root() { - let iter = AllPageNumbersBtreeIter::new( - header.root, - definition.fixed_key_size, - definition.fixed_value_size, - self.mem.clone(), - )?; - let mut freed_pages = self.freed_pages.lock().unwrap(); - for page_number in iter { - freed_pages.push(page_number?); + if definition.table_type == TableType::Normal { + if let Some(header) = definition.get_root() { + let iter = AllPageNumbersBtreeIter::new( + header.root, + definition.fixed_key_size, + definition.fixed_value_size, + self.mem.clone(), + )?; + let mut freed_pages = self.freed_pages.lock().unwrap(); + for page_number in iter { + freed_pages.push(page_number?); + } } + } else if definition.table_type == TableType::Multimap { + if let Some(header) = definition.get_root() { + let mut freed_pages = self.freed_pages.lock().unwrap(); + // Delete all the pages in the subtrees + let table_pages_iter = AllPageNumbersBtreeIter::new( + header.root, + definition.get_fixed_key_size(), + DynamicCollection::<()>::fixed_width_with( + definition.get_fixed_value_size(), + ), + self.mem.clone(), + )?; + for table_page in table_pages_iter { + let page = self.mem.get_page(table_page?)?; + let subtree_roots = parse_subtree_roots( + &page, + definition.get_fixed_key_size(), + definition.get_fixed_value_size(), + ); + for subtree_header in subtree_roots { + let sub_root_iter = AllPageNumbersBtreeIter::new( + subtree_header.root, + definition.get_fixed_value_size(), + <()>::fixed_width(), + self.mem.clone(), + )?; + for page in sub_root_iter { + freed_pages.push(page?); + } + } + } + // Now free the multimap table itself + let table_pages_iter = AllPageNumbersBtreeIter::new( + header.root, + definition.get_fixed_key_size(), + DynamicCollection::<()>::fixed_width_with( + definition.get_fixed_value_size(), + ), + self.mem.clone(), + )?; + for table_page in table_pages_iter { + freed_pages.push(table_page?); + } + } + } else { + unreachable!() } self.pending_table_updates.remove(name); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index e7ca97a3..cb44dc70 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1069,6 +1069,60 @@ fn regression23() { assert_eq!(allocated_pages, txn.stats().unwrap().allocated_pages()); } +#[test] +fn regression24() { + let tmpfile = create_tempfile(); + + let table_def: MultimapTableDefinition = MultimapTableDefinition::new("x"); + + let db = Database::create(tmpfile.path()).unwrap(); + let txn = db.begin_write().unwrap(); + { + // Touch the savepoints tables to be sure they get created, so that they occupy pages + let id = txn.persistent_savepoint().unwrap(); + txn.delete_persistent_savepoint(id).unwrap(); + // List the savepoints to ensure the system table is created and occupies a page + #[allow(unused_must_use)] + { + txn.list_persistent_savepoints().unwrap(); + } + let mut table = txn.open_table(U64_TABLE).unwrap(); + table.insert(0, 0).unwrap(); + } + txn.commit().unwrap(); + + let txn = db.begin_write().unwrap(); + { + txn.delete_table(U64_TABLE).unwrap(); + } + txn.commit().unwrap(); + + // Extra commit to finalize the cleanup of the freed pages + let txn = db.begin_write().unwrap(); + txn.commit().unwrap(); + + let txn = db.begin_write().unwrap(); + let allocated_pages = txn.stats().unwrap().allocated_pages(); + { + let mut table = txn.open_multimap_table(table_def).unwrap(); + table.insert(0, 0).unwrap(); + } + txn.commit().unwrap(); + + let txn = db.begin_write().unwrap(); + { + txn.delete_multimap_table(table_def).unwrap(); + } + txn.commit().unwrap(); + + // Extra commit to finalize the cleanup of the freed pages. + // There was a bug where deleting a multimap table leaked pages + db.begin_write().unwrap().commit().unwrap(); + + let txn = db.begin_write().unwrap(); + assert_eq!(allocated_pages, txn.stats().unwrap().allocated_pages()); +} + #[test] fn check_integrity_clean() { let tmpfile = create_tempfile();