Skip to content

Commit

Permalink
Fix potential corruption after restoring a savepoint
Browse files Browse the repository at this point in the history
If the file had grown enough that the region tracker had to be
reallocated the restoration would free the tracker. This could lead to
corruption, if that page was allocated for other use, when the tracker
then got flushed to the same location

This would only occur in real world usage on very large database files, since the default region
tracker is sized for 4TiB
  • Loading branch information
cberner committed Jul 28, 2024
1 parent a41f3ff commit e39e0f0
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,12 +709,14 @@ impl WriteTransaction {
.pages_allocated_since_raw_state(savepoint.get_regional_allocator_states());

// We don't want to rollback the system tree, so keep any pages it references
let referenced_by_system_tree = self
let mut whitelist = self
.system_tables
.lock()
.unwrap()
.table_tree
.all_referenced_pages()?;
// The tracker page could have changed too. Don't erase it.
whitelist.insert(self.mem.region_tracker_page());

// Find the oldest transaction in the current freed tree, for use below. We do this before
// freeing pages to ensure that this tree is still valid
Expand All @@ -732,7 +734,7 @@ impl WriteTransaction {

let mut freed_pages = vec![];
for page in allocated_since_savepoint {
if referenced_by_system_tree.contains(&page) {
if whitelist.contains(&page) {
continue;
}
freed_pages.push(page);
Expand Down Expand Up @@ -773,7 +775,7 @@ impl WriteTransaction {
// Re-process the freed pages, but ignore any that would be double-frees
if self.mem.is_allocated(page)
&& !freed_pages_hash.contains(&page)
&& !referenced_by_system_tree.contains(&page)
&& !whitelist.contains(&page)
{
freed_pages.push(page);
freed_pages_hash.insert(page);
Expand Down
4 changes: 4 additions & 0 deletions src/tree_store/page_store/page_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,10 @@ impl TransactionalMemory {
result
}

pub(crate) fn region_tracker_page(&self) -> PageNumber {
self.state.lock().unwrap().header.region_tracker()
}

// Relocates the region tracker to a lower page, if possible
// Returns true if the page was moved
pub(crate) fn relocate_region_tracker(&self) -> Result<bool> {
Expand Down

0 comments on commit e39e0f0

Please sign in to comment.