diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 471d92c00..2e27f3937 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -122,20 +122,15 @@ impl<'r> Transaction<'r> { pub fn commit(mut self) -> Operation { let mut_repo = Arc::try_unwrap(self.repo.take().unwrap()).ok().unwrap(); - let index_store = mut_repo.base_repo().index_store(); + let base_repo = mut_repo.base_repo(); + let index_store = base_repo.index_store(); let (mut_index, mut_view) = mut_repo.consume(); - // TODO: There is a race here: We add the operation to the list of head - // operations (in mut_view.save()) before we associate the operation with the - // index. That means that there's a small risk that another process finds the - // new operation and does redundant work to calculate the. We should - // probably make mut_view.save() write the operation without recording - // the new head (and without removing the old head), so we can associate the - // operation id with the index before we mark the operation as a head. let index = index_store.write_index(mut_index).unwrap(); let operation = mut_view.save(self.description.clone(), self.start_time.clone()); index_store .associate_file_with_operation(&index, operation.id()) .unwrap(); + mut_view.update_op_heads(&operation); self.closed = true; operation } diff --git a/lib/src/view.rs b/lib/src/view.rs index 078cae4fa..a0b0a148c 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -567,7 +567,7 @@ impl MutableView { enforce_invariants(&self.store, &mut self.data); } - pub fn save(self, description: String, operation_start_time: Timestamp) -> Operation { + pub fn save(&self, description: String, operation_start_time: Timestamp) -> Operation { let view_id = self.op_store.write_view(&self.data).unwrap(); let mut operation_metadata = OperationMetadata::new(description); operation_metadata.start_time = operation_start_time; @@ -577,11 +577,7 @@ impl MutableView { metadata: operation_metadata, }; let new_op_head_id = self.op_store.write_operation(&operation).unwrap(); - let operation = Operation::new(self.op_store.clone(), new_op_head_id, operation); - - self.update_op_heads(&operation); - - operation + Operation::new(self.op_store.clone(), new_op_head_id, operation) } pub fn update_op_heads(&self, op: &Operation) {