From 6083f209b4cfdaaf2bd838f25b5980b14925eb15 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Sat, 27 Apr 2024 17:10:24 -0700 Subject: [PATCH 1/2] Fix panic in compact() when read transaction is in progress --- src/db.rs | 7 +++++++ src/error.rs | 17 +++++++++++++++++ tests/integration_tests.rs | 30 ++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/db.rs b/src/db.rs index 55e0d413..3dbdc34e 100644 --- a/src/db.rs +++ b/src/db.rs @@ -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 { + 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())?; diff --git a/src/error.rs b/src/error.rs index 7be38ff2..46e97d48 100644 --- a/src/error.rs +++ b/src/error.rs @@ -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), } @@ -295,6 +297,7 @@ impl From for Error { match err { CompactionError::PersistentSavepointExists => Error::PersistentSavepointExists, CompactionError::EphemeralSavepointExists => Error::EphemeralSavepointExists, + CompactionError::TransactionInProgress => Error::TransactionInProgress, CompactionError::Storage(storage) => storage.into(), } } @@ -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), } } @@ -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 @@ -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.") } diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 6c87fe7f..408a19a7 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -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}; @@ -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(); From 94cc59009146a1b48068fbad1cb3dde9fd245e74 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Sat, 27 Apr 2024 17:32:48 -0700 Subject: [PATCH 2/2] Fix fuzzer on aarch64 OSX CI --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 87b6ddbd..69b94a48 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,6 +46,12 @@ jobs: components: rustfmt, clippy default: true + - name: OSX x86 rust + if: startsWith(matrix.os, 'macos') + run: | + # For some reason this is required to run the fuzzer on OSX + rustup target add x86_64-apple-darwin + - name: Install cargo-deny if: steps.rust-cache.outputs.cache-hit != 'true' run: rustup run --install 1.74 cargo install --force --version 0.14.17 cargo-deny --locked