From f0467b1be75029085d147dbda24cc27502fa7aec Mon Sep 17 00:00:00 2001 From: Alex Keizer Date: Fri, 18 Nov 2022 18:16:17 +0100 Subject: [PATCH] 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. --- src/rt/object.rs | 55 +++++++++++++++++++++++++++++++++++++++++++-- src/rt/path.rs | 10 ++++----- tests/cell_reuse.rs | 20 +++++++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 tests/cell_reuse.rs diff --git a/src/rt/object.rs b/src/rt/object.rs index 87959eb1..e36a15cf 100644 --- a/src/rt/object.rs +++ b/src/rt/object.rs @@ -15,8 +15,15 @@ use serde::{Deserialize, Serialize}; pub(super) struct Store { /// Stored state for all objects. entries: Vec, + + /// A unique id for this store, used to track references + id: StoreId, } +#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] +#[cfg_attr(feature = "checkpoint", derive(Serialize, Deserialize))] +pub(super) struct StoreId(usize); + pub(super) trait Object: Sized { type Entry; @@ -40,6 +47,10 @@ pub(super) struct Ref { /// 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, } @@ -147,6 +158,7 @@ impl Store { pub(super) fn with_capacity(capacity: usize) -> Store { Store { entries: Vec::with_capacity(capacity), + id: StoreId::new(), } } @@ -162,6 +174,10 @@ impl Store { self.entries.reserve_exact(additional); } + pub(super) fn get_id(&self) -> StoreId { + self.id + } + /// Insert an object into the store pub(super) fn insert(&mut self, item: O) -> Ref where @@ -172,6 +188,7 @@ impl Store { Ref { index, + store_id: self.id, _p: PhantomData, } } @@ -183,18 +200,22 @@ impl Store { pub(crate) fn clear(&mut self) { self.entries.clear(); + self.id = StoreId::new(); } pub(super) fn iter_ref(&self) -> impl DoubleEndedIterator> + '_ where O: Object, { + 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, }) } @@ -259,11 +280,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 Ref { /// Erase the type marker pub(super) fn erase(self) -> Ref<()> { Ref { index: self.index, + store_id: self.store_id, _p: PhantomData, } } @@ -274,8 +311,18 @@ impl Ref { } impl Ref { + /// Panic if the reference belongs to a different store + fn assert_store_id(self, store: &Store) { + 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 { + self.assert_store_id(&store); T::get_ref(&store.entries[self.index]) .expect("[loom internal bug] unexpected object stored at reference") } @@ -283,6 +330,7 @@ impl Ref { /// Get a mutable reference to the object associated with this reference /// from the store pub(super) fn get_mut(self, store: &mut Store) -> &mut T { + self.assert_store_id(&store); T::get_mut(&mut store.entries[self.index]) .expect("[loom internal bug] unexpected object stored at reference") } @@ -290,9 +338,10 @@ impl Ref { 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 +352,7 @@ impl Ref { { T::get_ref(&store.entries[self.index]).map(|_| Ref { index: self.index, + store_id: self.store_id, _p: PhantomData, }) } @@ -312,6 +362,7 @@ impl Clone for Ref { fn clone(&self) -> Ref { Ref { index: self.index, + store_id: self.store_id, _p: PhantomData, } } diff --git a/src/rt/path.rs b/src/rt/path.rs index 2334e1d5..194b1696 100644 --- a/src/rt/path.rs +++ b/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::(&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::(&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::(&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::(&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); diff --git a/tests/cell_reuse.rs b/tests/cell_reuse.rs new file mode 100644 index 00000000..81e668e2 --- /dev/null +++ b/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(); + }); +}