RcKey

storage/src/lib.rs

@@ -1,6 +1,7 @@
 mod memory_backend;
 mod storage;
 
+mod rc_key;
 mod serde_rc;
 mod sled_backend;
 mod storage_backend;

storage/src/rc_key.rs

@@ -0,0 +1,49 @@
+use std::any::Any;
+use std::hash::{Hash, Hasher};
+use std::rc::Rc;
+
+/**
+ * A wrapper around Rc suitable for use as a key in a HashMap.
+ *
+ * This fixes the issue with using the pointer as the key, since the pointer might be reused. By
+ * using RcKey, we still effectively use the pointer as the key, but we also hold a reference to the
+ * rc, so it cannot be dropped and reused, which would create a false association.
+ */
+pub struct RcKey(Rc<dyn Any>);
+
+impl RcKey {
+  pub fn from<T: 'static>(value: Rc<T>) -> Self {
+    RcKey(value as Rc<dyn Any>)
+  }
+}
+
+impl PartialEq for RcKey {
+  fn eq(&self, other: &Self) -> bool {
+    // This version:
+    //   Rc::ptr_eq(&self.0, &other.0)
+    //
+    // results in this feedback from clippy:
+    //   comparing trait object pointers compares a non-unique vtable address
+    //   consider extracting and comparing data pointers only
+    //
+    // The implementation below is an attempt to follow the advice of comparing data pointers, but
+    // I'm not sure whether this is a correct fix.
+    //
+    // For our purposes (caching), I suspect that even if this issue isn't resolved it is
+    // acceptable, since an alternative address would only reduce the cache hits, not cause
+    // incorrect behavior.
+
+    let self_ptr = Rc::as_ptr(&self.0) as *const ();
+    let other_ptr = Rc::as_ptr(&other.0) as *const ();
+
+    self_ptr == other_ptr
+  }
+}
+
+impl Eq for RcKey {}
+
+impl Hash for RcKey {
+  fn hash<H: Hasher>(&self, state: &mut H) {
+    Rc::as_ptr(&self.0).hash(state);
+  }
+}

storage/src/storage_ops.rs

@@ -4,6 +4,7 @@ use rand::thread_rng;
 use serde::{Deserialize, Serialize};
 
 use crate::{
+  rc_key::RcKey,
   storage_backend::StorageBackendHandle,
   storage_ptr::{StorageEntryPtr, StorageHeadPtr, StoragePtr},
   storage_val::StorageVal,
@@ -71,7 +72,7 @@ where
   }
 
   fn store_with_replacements(&mut self, value: &StorageVal) -> Result<StorageEntryPtr, E> {
-    let mut cache = HashMap::<u64, StorageEntryPtr>::new();
+    let mut cache = HashMap::<RcKey, StorageEntryPtr>::new();
 
     if let Some(key) = value
       .point

storage/src/storage_val.rs

@@ -3,6 +3,7 @@ use std::rc::Rc;
 
 use serde::{Deserialize, Serialize};
 
+use crate::rc_key::RcKey;
 use crate::serde_rc::{deserialize_rc, serialize_rc};
 use crate::storage_ops::StorageOps;
 use crate::storage_ptr::StorageEntryPtr;
@@ -55,7 +56,7 @@ impl StoragePoint {
     &self,
     tx: &mut SO,
     refs: &Rc<Vec<StorageEntryPtr>>,
-    cache: &mut HashMap<u64, StorageEntryPtr>,
+    cache: &mut HashMap<RcKey, StorageEntryPtr>,
   ) -> Result<Option<StorageEntryPtr>, E> {
     if let Some(id) = self.cache_id() {
       if let Some(key) = cache.get(&id) {
@@ -83,7 +84,7 @@ impl StoragePoint {
               refs: refs.clone(),
             },
             cache,
-            cache_id(arr),
+            RcKey::from(arr.clone()),
           )?);
         }
 
@@ -113,18 +114,18 @@ impl StoragePoint {
             refs: Rc::new(new_refs),
           },
           cache,
-          cache_id(arr),
+          RcKey::from(arr.clone()),
         )?)
       }
       StoragePoint::Ref(_) => None,
     })
   }
 
-  fn cache_id(&self) -> Option<u64> {
+  fn cache_id(&self) -> Option<RcKey> {
     match &self {
       StoragePoint::Void => None,
       StoragePoint::Number(_) => None,
-      StoragePoint::Array(arr) => Some(cache_id(arr)),
+      StoragePoint::Array(arr) => Some(RcKey::from(arr.clone())),
       StoragePoint::Ref(_) => None,
     }
   }
@@ -175,15 +176,11 @@ impl StorageEntry {
   }
 }
 
-fn cache_id<T>(rc: &Rc<T>) -> u64 {
-  rc.as_ref() as *const T as u64
-}
-
 fn cache_and_store<E, SO: StorageOps<E>>(
   tx: &mut SO,
   val: &StorageVal,
-  cache: &mut HashMap<u64, StorageEntryPtr>,
-  id: u64,
+  cache: &mut HashMap<RcKey, StorageEntryPtr>,
+  id: RcKey,
 ) -> Result<StorageEntryPtr, E> {
   let key = tx.store(val)?;