Skip to content

Commit

Permalink
Track which reference belongs to with store
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alexkeizer committed Dec 8, 2022
1 parent f15f931 commit f0467b1
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 7 deletions.
55 changes: 53 additions & 2 deletions src/rt/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,15 @@ 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)]
#[cfg_attr(feature = "checkpoint", derive(Serialize, Deserialize))]
pub(super) struct StoreId(usize);

pub(super) trait Object: Sized {
type Entry;

Expand All @@ -40,6 +47,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>,
}

Expand Down Expand Up @@ -147,6 +158,7 @@ impl<T> Store<T> {
pub(super) fn with_capacity(capacity: usize) -> Store<T> {
Store {
entries: Vec::with_capacity(capacity),
id: StoreId::new(),
}
}

Expand All @@ -162,6 +174,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
Expand All @@ -172,6 +188,7 @@ impl<T> Store<T> {

Ref {
index,
store_id: self.id,
_p: PhantomData,
}
}
Expand All @@ -183,18 +200,22 @@ 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,
})
}
Expand Down Expand Up @@ -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<T> Ref<T> {
/// Erase the type marker
pub(super) fn erase(self) -> Ref<()> {
Ref {
index: self.index,
store_id: self.store_id,
_p: PhantomData,
}
}
Expand All @@ -274,25 +311,37 @@ 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,
}
}
Expand All @@ -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,
})
}
Expand All @@ -312,6 +362,7 @@ impl<T> Clone for Ref<T> {
fn clone(&self) -> Ref<T> {
Ref {
index: self.index,
store_id: self.store_id,
_p: PhantomData,
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/rt/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 20 additions & 0 deletions tests/cell_reuse.rs
Original file line number Diff line number Diff line change
@@ -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();
});
}

0 comments on commit f0467b1

Please sign in to comment.