Skip to content

Commit 531d52d

Browse files
committed
Keep removed objects cached on persist failure
Check for the object first, remove it from the backing store, and only then delete it from the in-memory map. This finding was discovered by Project Loupe AI-Assisted-By: OpenAI Codex
1 parent e3793b8 commit 531d52d

1 file changed

Lines changed: 13 additions & 2 deletions

File tree

src/data_store.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ where
113113

114114
pub(crate) async fn remove(&self, id: &SO::Id) -> Result<(), Error> {
115115
let _guard = self.mutation_lock.lock().await;
116-
let removed = { self.objects.lock().expect("lock").remove(id).is_some() };
117-
if removed {
116+
let should_remove = { self.objects.lock().expect("lock").contains_key(id) };
117+
if should_remove {
118118
let store_key = id.encode_to_hex_str();
119119
KVStore::remove(
120120
&*self.kv_store,
@@ -135,6 +135,7 @@ where
135135
);
136136
Error::PersistenceFailed
137137
})?;
138+
self.objects.lock().expect("lock").remove(id);
138139
}
139140
Ok(())
140141
}
@@ -421,4 +422,14 @@ mod tests {
421422
assert_eq!(Err(Error::PersistenceFailed), data_store.update(update).await);
422423
assert_eq!(Some(object), data_store.get(&id));
423424
}
425+
426+
#[tokio::test]
427+
async fn remove_does_not_mutate_memory_if_persist_fails() {
428+
let id = TestObjectId { id: [42u8; 4] };
429+
let object = TestObject { id, data: [23u8; 3] };
430+
let data_store = new_failing_data_store(vec![object]);
431+
432+
assert_eq!(Err(Error::PersistenceFailed), data_store.remove(&id).await);
433+
assert_eq!(Some(object), data_store.get(&id));
434+
}
424435
}

0 commit comments

Comments
 (0)