Fix page leak in delete_multimap_table()

src/tree_store/table_tree.rs

@@ -796,17 +796,65 @@ impl<'txn> TableTreeMut<'txn> {
         table_type: TableType,
     ) -> Result<bool, TableError> {
         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);

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<u64, u64> = 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();