Skip to content

Commit

Permalink
Fix crash during I/O error handling
Browse files Browse the repository at this point in the history
If an I/O occurring during commit(), the transaction would then try to
abort() which might panic since the data structures are left in an
inconsistent state
  • Loading branch information
cberner committed May 9, 2023
1 parent eedcc5e commit 550bcff
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ impl<'db> WriteTransaction<'db> {
/// All writes performed in this transaction will be visible to future transactions, and are
/// durable as consistent with the [`Durability`] level set by [`Self::set_durability`]
pub fn commit(mut self) -> Result {
// Set completed flag first, so that we don't go through the abort() path on drop, if this fails
self.completed = true;
self.table_tree
.write()
.unwrap()
Expand All @@ -476,7 +478,6 @@ impl<'db> WriteTransaction<'db> {
Durability::Paranoid => self.durable_commit(false, true)?,
}

self.completed = true;
#[cfg(feature = "logging")]
info!(
"Finished commit of transaction id={:?}",
Expand All @@ -490,6 +491,8 @@ impl<'db> WriteTransaction<'db> {
///
/// All writes performed in this transaction will be rolled back
pub fn abort(mut self) -> Result {
// Set completed flag first, so that we don't go through the abort() path on drop, if this fails
self.completed = true;
self.abort_inner()
}

Expand All @@ -498,7 +501,6 @@ impl<'db> WriteTransaction<'db> {
info!("Aborting transaction id={:?}", self.transaction_id);
self.table_tree.write().unwrap().clear_table_root_updates();
self.mem.rollback_uncommitted_writes()?;
self.completed = true;
#[cfg(feature = "logging")]
info!("Finished abort of transaction id={:?}", self.transaction_id);
Ok(())
Expand Down
1 change: 1 addition & 0 deletions src/tree_store/page_store/page_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ impl TransactionalMemory {
"Dirty pages outstanding: {dirty_pages:?}"
);
}
assert!(!self.needs_recovery.load(Ordering::Acquire));
let mut state = self.state.lock().unwrap();
// The layout to restore
let (restore, restore_tracker_page) = if self.read_from_secondary.load(Ordering::Acquire) {
Expand Down

0 comments on commit 550bcff

Please sign in to comment.