Skip to content

Commit

Permalink
Fix panic in compact() when read transaction is in progress
Browse files Browse the repository at this point in the history
  • Loading branch information
cberner committed Apr 28, 2024
1 parent 0d0e11e commit 19fc5b5
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
7 changes: 7 additions & 0 deletions src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,13 @@ impl Database {
///
/// Returns `true` if compaction was performed, and `false` if no futher compaction was possible
pub fn compact(&mut self) -> Result<bool, CompactionError> {
if self
.transaction_tracker
.oldest_live_read_transaction()
.is_some()
{
return Err(CompactionError::TransactionInProgress);
}
// Commit to free up any pending free pages
// Use 2-phase commit to avoid any possible security issues. Plus this compaction is going to be so slow that it doesn't matter
let mut txn = self.begin_write().map_err(|e| e.into_storage_error())?;
Expand Down
17 changes: 17 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ pub enum CompactionError {
PersistentSavepointExists,
/// A ephemeral savepoint exists
EphemeralSavepointExists,
/// A transaction is still in-progress
TransactionInProgress,
/// Error from underlying storage
Storage(StorageError),
}
Expand All @@ -295,6 +297,7 @@ impl From<CompactionError> for Error {
match err {
CompactionError::PersistentSavepointExists => Error::PersistentSavepointExists,
CompactionError::EphemeralSavepointExists => Error::EphemeralSavepointExists,
CompactionError::TransactionInProgress => Error::TransactionInProgress,
CompactionError::Storage(storage) => storage.into(),
}
}
Expand All @@ -321,6 +324,12 @@ impl Display for CompactionError {
"Ephemeral savepoint exists. Operation cannot be performed."
)
}
CompactionError::TransactionInProgress => {
write!(
f,
"A transaction is still in progress. Operation cannot be performed."
)
}
CompactionError::Storage(storage) => storage.fmt(f),
}
}
Expand Down Expand Up @@ -434,6 +443,8 @@ pub enum Error {
PersistentSavepointExists,
/// An Ephemeral savepoint exists
EphemeralSavepointExists,
/// A transaction is still in-progress
TransactionInProgress,
/// The Database is corrupted
Corrupted(String),
/// The database file is in an old file format and must be manually upgraded
Expand Down Expand Up @@ -551,6 +562,12 @@ impl Display for Error {
"Ephemeral savepoint exists. Operation cannot be performed."
)
}
Error::TransactionInProgress => {
write!(
f,
"A transaction is still in progress. Operation cannot be performed."
)
}
Error::InvalidSavepoint => {
write!(f, "Savepoint is invalid or cannot be created.")
}
Expand Down
30 changes: 28 additions & 2 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ use std::ops::RangeBounds;
use rand::prelude::SliceRandom;
use rand::Rng;
use redb::{
AccessGuard, Builder, Database, Durability, Key, MultimapRange, MultimapTableDefinition,
MultimapValue, Range, ReadableTable, ReadableTableMetadata, TableDefinition, TableStats, Value,
AccessGuard, Builder, CompactionError, Database, Durability, Key, MultimapRange,
MultimapTableDefinition, MultimapValue, Range, ReadableTable, ReadableTableMetadata,
TableDefinition, TableStats, Value,
};
use redb::{DatabaseError, ReadableMultimapTable, SavepointError, StorageError, TableError};

Expand Down Expand Up @@ -867,6 +868,31 @@ fn regression20() {
}
}

#[test]
fn regression21() {
let tmpfile = create_tempfile();

let mut db = Database::create(tmpfile.path()).unwrap();

let write_tx = db.begin_write().unwrap();
let read_tx = db.begin_read().unwrap();

let mut write_table = write_tx
.open_table::<&str, &str>(TableDefinition::new("example"))
.unwrap();

write_table.insert("example", "example").unwrap();

drop(write_table);

write_tx.commit().unwrap();
assert!(matches!(
db.compact().unwrap_err(),
CompactionError::TransactionInProgress
));
drop(read_tx);
}

#[test]
fn check_integrity_clean() {
let tmpfile = create_tempfile();
Expand Down

0 comments on commit 19fc5b5

Please sign in to comment.