Track which reference belongs to with store

Add a unique id to stores, and keep track of this id
in references.
When a reference is used with a different store, this will
panic with a message explaining that the user might be
trying to reuse object across executions.

Author: alexkeizer

src/rt/object.rs

@@ -15,8 +15,16 @@ use serde::{Deserialize, Serialize};
 pub(super) struct Store<T = Entry> {
     /// Stored state for all objects.
     entries: Vec<T>,
+
+    /// A unique id for this store, used to track references
+    id: StoreId,
 }
 
+
+#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)]
+pub(super) struct StoreId(usize);
+
+
 pub(super) trait Object: Sized {
     type Entry;
 
@@ -40,6 +48,10 @@ pub(super) struct Ref<T = ()> {
     /// Index in the store
     index: usize,
 
+    /// Id of the store that created this reference. Optional, since `Ref`s can also be created from
+    /// a bare `usize` index
+    store_id: StoreId,
+
     _p: PhantomData<T>,
 }
 
@@ -147,6 +159,7 @@ impl<T> Store<T> {
     pub(super) fn with_capacity(capacity: usize) -> Store<T> {
         Store {
             entries: Vec::with_capacity(capacity),
+            id: StoreId::new(),
         }
     }
 
@@ -162,6 +175,10 @@ impl<T> Store<T> {
         self.entries.reserve_exact(additional);
     }
 
+    pub(super) fn get_id(&self) -> StoreId {
+        self.id
+    }
+
     /// Insert an object into the store
     pub(super) fn insert<O>(&mut self, item: O) -> Ref<O>
     where
@@ -172,6 +189,7 @@ impl<T> Store<T> {
 
         Ref {
             index,
+            store_id: self.id,
             _p: PhantomData,
         }
     }
@@ -183,25 +201,29 @@ impl<T> Store<T> {
 
     pub(crate) fn clear(&mut self) {
         self.entries.clear();
+        self.id = StoreId::new();
     }
 
     pub(super) fn iter_ref<O>(&self) -> impl DoubleEndedIterator<Item = Ref<O>> + '_
     where
         O: Object<Entry = T>,
     {
+        let store_id = self.id;
+
         self.entries
             .iter()
             .enumerate()
             .filter(|(_, e)| O::get_ref(e).is_some())
-            .map(|(index, _)| Ref {
+            .map(move |(index, _)| Ref {
                 index,
+                store_id,
                 _p: PhantomData,
             })
     }
 
-    pub(super) fn iter_mut<'a, O>(&'a mut self) -> impl DoubleEndedIterator<Item = &mut O>
-    where
-        O: Object<Entry = T> + 'a,
+    pub(super) fn iter_mut<'a, O>(&'a mut self) -> impl DoubleEndedIterator<Item=&mut O>
+        where
+            O: Object<Entry=T> + 'a,
     {
         self.entries.iter_mut().filter_map(O::get_mut)
     }
@@ -259,11 +281,27 @@ impl Store {
     }
 }
 
+impl StoreId {
+    pub(crate) fn new() -> StoreId {
+        use std::sync::atomic::AtomicUsize;
+        use std::sync::atomic::Ordering::Relaxed;
+
+        // The number picked here is arbitrary. It is mostly to avoid collision
+        // with "zero" to aid with debugging.
+        static NEXT_ID: AtomicUsize = AtomicUsize::new(41_123_456);
+
+        let next = NEXT_ID.fetch_add(1, Relaxed);
+
+        StoreId(next)
+    }
+}
+
 impl<T> Ref<T> {
     /// Erase the type marker
     pub(super) fn erase(self) -> Ref<()> {
         Ref {
             index: self.index,
+            store_id: self.store_id,
             _p: PhantomData,
         }
     }
@@ -274,25 +312,38 @@ impl<T> Ref<T> {
 }
 
 impl<T: Object> Ref<T> {
+    /// Panic if the reference belongs to a different store
+    fn assert_store_id(self, store: &Store<T::Entry>) {
+        assert_eq!(
+            self.store_id,
+            store.id,
+            "Tried to access an object using a reference that belongs to a different store. \
+                This might indicate you are trying to reuse an object from an earlier execution"
+        )
+    }
+
     /// Get a reference to the object associated with this reference from the store
     pub(super) fn get(self, store: &Store<T::Entry>) -> &T {
+        self.assert_store_id(&store);
         T::get_ref(&store.entries[self.index])
             .expect("[loom internal bug] unexpected object stored at reference")
     }
 
     /// Get a mutable reference to the object associated with this reference
     /// from the store
     pub(super) fn get_mut(self, store: &mut Store<T::Entry>) -> &mut T {
+        self.assert_store_id(&store);
         T::get_mut(&mut store.entries[self.index])
             .expect("[loom internal bug] unexpected object stored at reference")
     }
 }
 
 impl Ref {
     /// Convert a store index `usize` into a ref
-    pub(super) fn from_usize(index: usize) -> Ref {
+    pub(super) fn from_usize(index: usize, store_id: StoreId) -> Ref {
         Ref {
             index,
+            store_id,
             _p: PhantomData,
         }
     }
@@ -303,6 +354,7 @@ impl Ref {
     {
         T::get_ref(&store.entries[self.index]).map(|_| Ref {
             index: self.index,
+            store_id: self.store_id,
             _p: PhantomData,
         })
     }
@@ -312,6 +364,7 @@ impl<T> Clone for Ref<T> {
     fn clone(&self) -> Ref<T> {
         Ref {
             index: self.index,
+            store_id: self.store_id,
             _p: PhantomData,
         }
     }

src/rt/path.rs

@@ -166,7 +166,7 @@ impl Path {
     pub(super) fn branch_load(&mut self) -> usize {
         assert!(!self.is_traversed(), "[loom internal bug]");
 
-        let load = object::Ref::from_usize(self.pos)
+        let load = object::Ref::from_usize(self.pos, self.branches.get_id())
             .downcast::<Load>(&self.branches)
             .expect("Reached unexpected exploration state. Is the model fully deterministic?")
             .get(&self.branches);
@@ -184,7 +184,7 @@ impl Path {
             self.branches.insert(Spurious(false));
         }
 
-        let spurious = object::Ref::from_usize(self.pos)
+        let spurious = object::Ref::from_usize(self.pos, self.branches.get_id())
             .downcast::<Spurious>(&self.branches)
             .expect("Reached unexpected exploration state. Is the model fully deterministic?")
             .get(&self.branches)
@@ -274,7 +274,7 @@ impl Path {
             schedule.preemptions = preemptions;
         }
 
-        let schedule = object::Ref::from_usize(self.pos)
+        let schedule = object::Ref::from_usize(self.pos, self.branches.get_id())
             .downcast::<Schedule>(&self.branches)
             .expect("Reached unexpected exploration state. Is the model fully deterministic?")
             .get(&self.branches);
@@ -290,7 +290,7 @@ impl Path {
     }
 
     pub(super) fn backtrack(&mut self, point: usize, thread_id: thread::Id) {
-        let schedule = object::Ref::from_usize(point)
+        let schedule = object::Ref::from_usize(point, self.branches.get_id())
             .downcast::<Schedule>(&self.branches)
             .unwrap()
             .get_mut(&mut self.branches);
@@ -344,7 +344,7 @@ impl Path {
         // This is depth-first tree traversal.
         //
         for last in (0..self.branches.len()).rev() {
-            let last = object::Ref::from_usize(last);
+            let last = object::Ref::from_usize(last, self.branches.get_id());
 
             // Remove all objects that were created **after** this branch
             self.branches.truncate(last);

tests/cell_reuse.rs

@@ -0,0 +1,20 @@
+#![deny(warnings, rust_2018_idioms)]
+
+use loom::cell::Cell;
+
+thread_local! {
+    static ACTIVE_FRAME: Cell<()> = Cell::new(());
+}
+
+#[test]
+#[should_panic="Tried to access an object using a reference that belongs to a different store. This might indicate you are trying to reuse an object from an earlier execution"]
+fn test_cell_reuse() {
+    loom::model(|| {
+        let handle_a = loom::thread::spawn(|| {
+            let _ = ACTIVE_FRAME.with(Cell::get);
+        });
+        let handle_b = loom::thread::spawn(|| ());
+        handle_a.join().unwrap();
+        handle_b.join().unwrap();
+    });
+}