Add lock sync primitives #52
Conversation
| #[test] | ||
| fn modify_item_with_locks() { | ||
| let mut store = MockStorage::new(); | ||
| const PERSON: Item<Lockable<Person>> = Item::new("person"); |
There was a problem hiding this comment.
Interesting functionality of a persistent lock. This can prove very handy.
| .unwrap_err(); | ||
| assert_eq!(err, PersonError::Lock(LockError::WriteLocked)); | ||
|
|
||
| // We can get count (kind of edge case bug, but I don't think we can change this or it matters) |
There was a problem hiding this comment.
Technically, this is correct. The map itself is unlocked, and so, its state can change.
The same with removal of a locked item, I guess? Would be good to add a test for it, just to showcase it.
There was a problem hiding this comment.
Unless we consider removing a locked item an error, and use a wrapper around remove to try and enforce that.
There was a problem hiding this comment.
Unless we consider removing a locked item an error, and use a wrapper around remove to try and enforce that.
Yeah, it seemed a new LockedMap type would be a whole lot of work, so I was trying to figure out how to do this with minimal intervention.
I think it is correct that count works, but remove should definitely be prohibited. Any ideas how to do so without copying over the entire Map API?
There was a problem hiding this comment.
Rust composition over inheritance makes writing this kind of minimally modified containers a pain.
The only way I see around a LockedMap / LockableMap and similar is a Lockable::remove method. It feels unnatural, and enforcement will be responsibility of the user (as there's no way to avoid calling the base / original remove), but it's perhaps preferable.
There was a problem hiding this comment.
I will add such a method as helper.
Maybe another PR to add some sort of LockableMap alternative which enforces everything
Based on the theoretical work in #49
Define a
Lockable<T>type that can be stored in anItemorMapand manages pending read or write locks in storage,so as to hold them over multiple blocks until an IBC ack comes in.
The "normal" use now needs to be via
val.read()?orval.write()?to get&Tor&mut T, so a slight change to calling code, but not too bad. And there are new methods to lock or unlock it which can control whether the previous are allowed.Also, usage in higher-level constructs (like
Map::range) should enforce this and error if a single element has a write-lock.