ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

transaction: fix (mostly harmless) race where index can get re-calculated

Browse source
This commit is contained in:
Martin von Zweigbergk 2021-03-10 15:07:59 -08:00
parent 47a7cf7101
commit 9ee521d9d3
2 changed files with 5 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
lib/src/transaction.rs
View file

@ -122,20 +122,15 @@ impl<'r> Transaction<'r> {
pub fn commit(mut self) -> Operation { pub fn commit(mut self) -> Operation {
let mut_repo = Arc::try_unwrap(self.repo.take().unwrap()).ok().unwrap(); 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(); 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 index = index_store.write_index(mut_index).unwrap();
let operation = mut_view.save(self.description.clone(), self.start_time.clone()); let operation = mut_view.save(self.description.clone(), self.start_time.clone());
index_store index_store
.associate_file_with_operation(&index, operation.id()) .associate_file_with_operation(&index, operation.id())
.unwrap(); .unwrap();
mut_view.update_op_heads(&operation);
self.closed = true; self.closed = true;
operation operation
} }

8
lib/src/view.rs
View file

@ -567,7 +567,7 @@ impl MutableView {
enforce_invariants(&self.store, &mut self.data); 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 view_id = self.op_store.write_view(&self.data).unwrap();
let mut operation_metadata = OperationMetadata::new(description); let mut operation_metadata = OperationMetadata::new(description);
operation_metadata.start_time = operation_start_time; operation_metadata.start_time = operation_start_time;
@ -577,11 +577,7 @@ impl MutableView {
metadata: operation_metadata, metadata: operation_metadata,
}; };
let new_op_head_id = self.op_store.write_operation(&operation).unwrap(); 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); Operation::new(self.op_store.clone(), new_op_head_id, operation)
self.update_op_heads(&operation);
operation
} }
pub fn update_op_heads(&self, op: &Operation) { pub fn update_op_heads(&self, op: &Operation) {