Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track which reference belongs to which store #292

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
});
}