Conversation
| Some("test_value2".to_string()) | ||
| ); | ||
| assert_eq!(store.read("test3").unwrap(), None); | ||
| store.rollback_transaction(second_transaction_id).unwrap(); |
There was a problem hiding this comment.
This was wrong right, that is why you remove it ?
There was a problem hiding this comment.
The rollback wasn´t part of the test. It was used to solve a leaking memory issue with unclosed transactions when Storage went out of scope. This was fixed with the implementation of the Drop Trait
| impl Drop for Storage { | ||
| fn drop(&mut self) { | ||
| let mut map = self.transactions.borrow_mut(); | ||
| map.clear(); |
There was a problem hiding this comment.
Should drop implement transaction/rollback?
There was a problem hiding this comment.
The intention for drop is remove transactions?
There was a problem hiding this comment.
Maybe we can call it transaction_drop
There was a problem hiding this comment.
It's is ok, when the object is removed, the transaction is automatically rolled back. The change is to make sure that on shutdown, even if there is a transaction open it does not crash.
But I would add a comment explaining why this is here
There was a problem hiding this comment.
The Drop Trait was defined because if you left transactions unclosed when Storage went out of scope there was a leaking memory issue. I can´t change the name of the function, it's defined by the trait, but I can document the function explaining why it's needed.
3 participants
No description provided.