trying out Mutex instead of Cell

Author: srdtrk

packages/storage/src/append_store.rs

@@ -4,7 +4,7 @@
 //! This is achieved by storing each item in a separate storage entry. A special key is reserved
 //! for storing the length of the collection so far.
 use std::any::type_name;
-use std::cell::Cell;
+use std::sync::Mutex;
 use std::{convert::TryInto};
 use std::marker::PhantomData;
 
@@ -26,7 +26,7 @@ pub struct AppendStore<'a, T, Ser = Bincode2>
     /// needed if any suffixes were added to the original namespace.
     /// therefore it is not necessarily same as the namespace.
     prefix: Option<Vec<u8>>,
-    length: Cell<Option<u32>>,
+    length: Mutex<Option<u32>>,
     item_type: PhantomData<T>,
     serialization_type: PhantomData<Ser>,
 }
@@ -37,7 +37,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser>{
         Self {
             namespace: prefix,
             prefix: None,
-            length: Cell::new(None),
+            length: Mutex::new(None),
             item_type: PhantomData,
             serialization_type: PhantomData,
         }
@@ -53,7 +53,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser>{
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
-            length: Cell::new(None),
+            length: Mutex::new(None),
             item_type: self.item_type,
             serialization_type: self.serialization_type,
         }
@@ -63,17 +63,18 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser>{
 impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     /// gets the length from storage, and otherwise sets it to 0
     pub fn get_len<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        match self.length.get() {
+        let mut may_len = self.length.lock().unwrap();
+        match *may_len {
             Some(len) => { Ok(len) },
             None => {
                 let len_key = [self.as_slice(), LEN_KEY].concat();
                 if let Some(len_vec) = storage.get(&len_key) {
                     let len_bytes = len_vec.as_slice().try_into().map_err(|err| StdError::parse_err("u32", err))?;
                     let len = u32::from_be_bytes(len_bytes);
-                    self.length.set(Some(len));
+                    *may_len = Some(len);
                     Ok(len)
                 } else {
-                    self.length.set(Some(0));
+                    *may_len = Some(0);
                     Ok(0)
                 }
             },
@@ -98,8 +99,10 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     /// Set the length of the collection
     fn set_len<S: Storage>(&self, storage: &mut S, len: u32) {
         let len_key = [self.as_slice(), LEN_KEY].concat();
-        self.length.set(Some(len));
         storage.set(&len_key, &len.to_be_bytes());
+
+        let mut may_len = self.length.lock().unwrap();
+        *may_len = Some(len);
     }
     /// Clear the collection
     pub fn clear<S: Storage>(&self, storage: &mut S) {
@@ -342,29 +345,29 @@ mod tests {
         let mut storage = MockStorage::new();
         let append_store: AppendStore<i32> = AppendStore::new(b"test");
 
-        assert_eq!(append_store.length, Cell::new(None));
+        assert!(append_store.length.lock().unwrap().eq(&None));
         assert_eq!(append_store.get_len(&mut storage)?, 0);
-        assert_eq!(append_store.length, Cell::new(Some(0)));
+        assert!(append_store.length.lock().unwrap().eq(&Some(0)));
 
         append_store.push(&mut storage, &1234)?;
         append_store.push(&mut storage, &2143)?;
         append_store.push(&mut storage, &3412)?;
         append_store.push(&mut storage, &4321)?;
-        assert_eq!(append_store.length, Cell::new(Some(4)));
+        assert!(append_store.length.lock().unwrap().eq(&Some(4)));
         assert_eq!(append_store.get_len(&mut storage)?, 4);
 
         assert_eq!(append_store.pop(&mut storage), Ok(4321));
         assert_eq!(append_store.pop(&mut storage), Ok(3412));
-        assert_eq!(append_store.length, Cell::new(Some(2)));
+        assert!(append_store.length.lock().unwrap().eq(&Some(2)));
         assert_eq!(append_store.get_len(&mut storage)?, 2);
 
         assert_eq!(append_store.pop(&mut storage), Ok(2143));
         assert_eq!(append_store.pop(&mut storage), Ok(1234));
-        assert_eq!(append_store.length, Cell::new(Some(0)));
+        assert!(append_store.length.lock().unwrap().eq(&Some(0)));
         assert_eq!(append_store.get_len(&mut storage)?, 0);
 
         assert!(append_store.pop(&mut storage).is_err());
-        assert_eq!(append_store.length, Cell::new(Some(0)));
+        assert!(append_store.length.lock().unwrap().eq(&Some(0)));
         assert_eq!(append_store.get_len(&mut storage)?, 0);
 
         Ok(())

packages/storage/src/deque_store.rs

@@ -5,7 +5,7 @@
 //! A special key is reserved for storing the length of the collection so far.
 //! Another special key is reserved for storing the offset of the collection.
 use std::any::type_name;
-use std::cell::Cell;
+use std::sync::Mutex;
 use std::{convert::TryInto};
 use std::marker::PhantomData;
 
@@ -28,8 +28,8 @@ pub struct DequeStore<'a, T, Ser = Bincode2>
     /// needed if any suffixes were added to the original namespace.
     /// therefore it is not necessarily same as the namespace.
     prefix: Option<Vec<u8>>,
-    length: Cell<Option<u32>>,
-    offset: Cell<Option<u32>>,
+    length: Mutex<Option<u32>>,
+    offset: Mutex<Option<u32>>,
     item_type: PhantomData<T>,
     serialization_type: PhantomData<Ser>,
 }
@@ -40,8 +40,8 @@ impl<'a, 'b, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser>
         Self {
             namespace: prefix,
             prefix: None,
-            length: Cell::new(None),
-            offset: Cell::new(None),
+            length: Mutex::new(None),
+            offset: Mutex::new(None),
             item_type: PhantomData,
             serialization_type: PhantomData,
         }
@@ -57,8 +57,8 @@ impl<'a, 'b, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser>
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
-            length: Cell::new(None),
-            offset: Cell::new(None),
+            length: Mutex::new(None),
+            offset: Mutex::new(None),
             item_type: self.item_type,
             serialization_type: self.serialization_type,
         }
@@ -68,12 +68,13 @@ impl<'a, 'b, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser>
 impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     /// gets the length from storage, and otherwise sets it to 0
     pub fn get_len<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        match self.length.get() {
+        let mut may_len = self.length.lock().unwrap();
+        match *may_len {
             Some(len) => { Ok(len) },
             None => {
                 match self._get_u32(storage, LEN_KEY) {
                     Ok(len) => {
-                        self.length.set(Some(len));
+                        *may_len = Some(len);
                         Ok(len)
                     },
                     Err(e) => { Err(e) },
@@ -83,12 +84,13 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     }
     /// gets the offset from storage, and otherwise sets it to 0
     pub fn get_off<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        match self.offset.get() {
+        let mut may_off = self.offset.lock().unwrap();
+        match *may_off {
             Some(len) => { Ok(len) },
             None => {
                 match self._get_u32(storage, OFFSET_KEY) {
                     Ok(len) => {
-                        self.offset.set(Some(len));
+                        *may_off = Some(len);
                         Ok(len)
                     },
                     Err(e) => { Err(e) },
@@ -128,12 +130,14 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     }
     /// Set the length of the collection
     fn set_len<S: Storage>(&self, storage: &mut S, len: u32) {
-        self.length.set(Some(len));
+        let mut may_len = self.length.lock().unwrap();
+        *may_len = Some(len);
         self._set_u32(storage, LEN_KEY, len)
     }
     /// Set the offset of the collection
     fn set_off<S: Storage>(&self, storage: &mut S, off: u32) {
-        self.offset.set(Some(off));
+        let mut may_off = self.offset.lock().unwrap();
+        *may_off = Some(off);
         self._set_u32(storage, OFFSET_KEY, off)
     }
     /// Set the length or offset of the collection

packages/storage/src/keymap.rs

@@ -1,5 +1,5 @@
 use std::any::type_name;
-use std::cell::{Cell};
+use std::sync::Mutex;
 use std::{convert::TryInto};
 use std::marker::PhantomData;
 
@@ -39,7 +39,7 @@ pub struct Keymap<'a, K, T, Ser = Bincode2>
     /// needed if any suffixes were added to the original namespace.
     /// therefore it is not necessarily same as the namespace.
     prefix: Option<Vec<u8>>,
-    length: Cell<Option<u32>>,
+    length: Mutex<Option<u32>>,
     key_type: PhantomData<K>,
     item_type: PhantomData<T>,
     serialization_type: PhantomData<Ser>,
@@ -51,7 +51,7 @@ impl<'a, K: Serialize + DeserializeOwned, T: Serialize + DeserializeOwned, Ser:
         Self {
             namespace: prefix,
             prefix: None,
-            length: Cell::new(None),
+            length: Mutex::new(None),
             key_type: PhantomData,
             item_type: PhantomData,
             serialization_type: PhantomData,
@@ -68,7 +68,7 @@ impl<'a, K: Serialize + DeserializeOwned, T: Serialize + DeserializeOwned, Ser:
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
-            length: Cell::new(None),
+            length: Mutex::new(None),
             key_type: self.key_type,
             item_type: self.item_type,
             serialization_type: self.serialization_type,
@@ -87,17 +87,18 @@ impl<'a, K: Serialize + DeserializeOwned, T: Serialize + DeserializeOwned, Ser:
     }
     /// get total number of objects saved
     pub fn get_len<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        match self.length.get() {
+        let mut may_len = self.length.lock().unwrap();
+        match *may_len {
             Some(length) => { Ok(length) },
             None => {
                 let len_key = [self.as_slice(), MAP_LENGTH].concat();
                 if let Some(len_vec) = storage.get(&len_key) {
                     let len_bytes = len_vec.as_slice().try_into().map_err(|err| StdError::parse_err("u32", err))?;
                     let len = u32::from_be_bytes(len_bytes);
-                    self.length.set(Some(len));
+                    *may_len = Some(len);
                     Ok(len)
                 } else {
-                    self.length.set(Some(0));
+                    *may_len = Some(0);
                     Ok(0)
                 }
             },
@@ -111,7 +112,10 @@ impl<'a, K: Serialize + DeserializeOwned, T: Serialize + DeserializeOwned, Ser:
     fn set_len<S: Storage>(&self, storage: &mut S, len: u32) -> StdResult<()> {
         let len_key = [self.as_slice(), MAP_LENGTH].concat();
         storage.set(&len_key, &len.to_be_bytes());
-        self.length.set(Some(len));
+
+        let mut may_len = self.length.lock().unwrap();
+        *may_len = Some(len);
+
         Ok(())
     }
     /// Used to get the indexes stored in the given page number
@@ -1262,36 +1266,36 @@ mod tests {
             number: 1111,
         };
 
-        assert_eq!(keymap.length, Cell::new(None));
+        assert!(keymap.length.lock().unwrap().eq(&None));
         assert_eq!(keymap.get_len(&storage)?, 0);
-        assert_eq!(keymap.length, Cell::new(Some(0)));
+        assert!(keymap.length.lock().unwrap().eq(&Some(0)));
 
         let key1 = "k1".to_string();
         let key2 = "k2".to_string();
 
         keymap.insert(&mut storage, &key1, foo1.clone())?;
         assert_eq!(keymap.get_len(&storage)?, 1);
-        assert_eq!(keymap.length, Cell::new(Some(1)));
+        assert!(keymap.length.lock().unwrap().eq(&Some(1)));
 
         // add another item
         keymap.insert(&mut storage, &key2, foo2.clone())?;
         assert_eq!(keymap.get_len(&storage)?, 2);
-        assert_eq!(keymap.length, Cell::new(Some(2)));
+        assert!(keymap.length.lock().unwrap().eq(&Some(2)));
 
         // remove item and check length
         keymap.remove(&mut storage, &key1)?;
         assert_eq!(keymap.get_len(&storage)?, 1);
-        assert_eq!(keymap.length, Cell::new(Some(1)));
+        assert!(keymap.length.lock().unwrap().eq(&Some(1)));
 
         // override item (should not change length)
         keymap.insert(&mut storage, &key2, foo1)?;
         assert_eq!(keymap.get_len(&storage)?, 1);
-        assert_eq!(keymap.length, Cell::new(Some(1)));
+        assert!(keymap.length.lock().unwrap().eq(&Some(1)));
 
         // remove item and check length
         keymap.remove(&mut storage, &key2)?;
         assert_eq!(keymap.get_len(&storage)?, 0);
-        assert_eq!(keymap.length, Cell::new(Some(0)));
+        assert!(keymap.length.lock().unwrap().eq(&Some(0)));
 
         Ok(())
     }