Skip to content

Commit ecd63ff

Browse files
committed
util/epoch: Add preliminary support for loom
This patch only adds support to parts of `utils` and to `epoch`. Some parts of `utils` had to be left out, since they rely on `AtomicUsize::new` being `const` (which it is not in `loom`). Other parts had to be left out due to the lack of `thread::Thread` in `loom`. All the parts needed for `epoch` were successfully moved to loom. For this initial patch, there are two loom tests, both in `epoch`. One is a simple test of defer_destroy while a pin is held, and the other is the Triber stack example. They both pass loom with `LOOM_MAX_PREEMPTIONS=3` and `LOOM_MAX_PREEMPTIONS=2`. The latter tests fewer possible interleavings, but completes in 13 minutes on my laptop rather than ~2 hours. I have added loom testing of `epoch` to CI as well. Note that the uses of `UnsafeCell` in `utils` have not been moved to `loom::cell::UnsafeCell`, as loom's `UnsafeCell` does not support `T: ?Sized`, which `AtomicCell` depends on. Fixes #486.
1 parent 33b9232 commit ecd63ff

24 files changed

Lines changed: 401 additions & 39 deletions

File tree

.github/workflows/ci.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,14 @@ jobs:
6060
run: rustup update stable && rustup default stable
6161
- name: rustfmt
6262
run: ./ci/rustfmt.sh
63+
64+
# Run loom tests.
65+
loom:
66+
name: loom
67+
runs-on: ubuntu-latest
68+
steps:
69+
- uses: actions/checkout@master
70+
- name: Install Rust
71+
run: rustup update stable && rustup default stable
72+
- name: loom
73+
run: ./ci/crossbeam-epoch-loom.sh

ci/crossbeam-epoch-loom.sh

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/bin/bash
2+
3+
cd "$(dirname "$0")"/../crossbeam-epoch
4+
set -ex
5+
6+
export RUSTFLAGS="-D warnings --cfg=loom"
7+
8+
env LOOM_MAX_PREEMPTIONS=2 cargo test --test loom --features sanitize --release -- --nocapture

crossbeam-epoch/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ sanitize = [] # Makes it more likely to trigger any potential data races.
4040
cfg-if = "0.1.10"
4141
memoffset = "0.5.1"
4242

43+
[target.'cfg(loom)'.dependencies]
44+
loom = "0.3.2"
45+
4346
[dependencies.crossbeam-utils]
4447
version = "0.7"
4548
path = "../crossbeam-utils"

crossbeam-epoch/src/atomic.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::concurrency::sync::atomic::AtomicUsize;
12
use alloc::boxed::Box;
23
use core::borrow::{Borrow, BorrowMut};
34
use core::cmp;
@@ -6,7 +7,7 @@ use core::marker::PhantomData;
67
use core::mem;
78
use core::ops::{Deref, DerefMut};
89
use core::ptr;
9-
use core::sync::atomic::{AtomicUsize, Ordering};
10+
use core::sync::atomic::Ordering;
1011

1112
use crate::guard::Guard;
1213
use crossbeam_utils::atomic::AtomicConsume;
@@ -150,6 +151,24 @@ impl<T> Atomic<T> {
150151
///
151152
/// let a = Atomic::<i32>::null();
152153
/// ```
154+
#[cfg(loom)]
155+
pub fn null() -> Atomic<T> {
156+
Self {
157+
data: AtomicUsize::new(0),
158+
_marker: PhantomData,
159+
}
160+
}
161+
162+
/// Returns a new null atomic pointer.
163+
///
164+
/// # Examples
165+
///
166+
/// ```
167+
/// use crossbeam_epoch::Atomic;
168+
///
169+
/// let a = Atomic::<i32>::null();
170+
/// ```
171+
#[cfg(not(loom))]
153172
pub const fn null() -> Atomic<T> {
154173
Self {
155174
data: AtomicUsize::new(0),
@@ -488,7 +507,14 @@ impl<T> Atomic<T> {
488507
/// }
489508
/// ```
490509
pub unsafe fn into_owned(self) -> Owned<T> {
491-
Owned::from_usize(self.data.into_inner())
510+
#[cfg(loom)]
511+
{
512+
Owned::from_usize(self.data.unsync_load())
513+
}
514+
#[cfg(not(loom))]
515+
{
516+
Owned::from_usize(self.data.into_inner())
517+
}
492518
}
493519
}
494520

@@ -1167,7 +1193,7 @@ impl<T> Default for Shared<'_, T> {
11671193
}
11681194
}
11691195

1170-
#[cfg(test)]
1196+
#[cfg(all(test, not(loom)))]
11711197
mod tests {
11721198
use super::Shared;
11731199

crossbeam-epoch/src/collector.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
///
1313
/// handle.pin().flush();
1414
/// ```
15-
use alloc::sync::Arc;
15+
use crate::concurrency::sync::Arc;
1616
use core::fmt;
1717

1818
use crate::guard::Guard;
@@ -103,7 +103,7 @@ impl fmt::Debug for LocalHandle {
103103
}
104104
}
105105

106-
#[cfg(test)]
106+
#[cfg(all(test, not(loom)))]
107107
mod tests {
108108
use std::mem;
109109
use std::sync::atomic::{AtomicUsize, Ordering};
@@ -145,9 +145,9 @@ mod tests {
145145
let a = Owned::new(7).into_shared(guard);
146146
guard.defer_destroy(a);
147147

148-
assert!(!(*(*guard.local).bag.get()).is_empty());
148+
assert!(!(*guard.local).bag.with(|b| (*b).is_empty()));
149149

150-
while !(*(*guard.local).bag.get()).is_empty() {
150+
while !(*guard.local).bag.with(|b| (*b).is_empty()) {
151151
guard.flush();
152152
}
153153
}
@@ -166,7 +166,7 @@ mod tests {
166166
let a = Owned::new(7).into_shared(guard);
167167
guard.defer_destroy(a);
168168
}
169-
assert!(!(*(*guard.local).bag.get()).is_empty());
169+
assert!(!(*guard.local).bag.with(|b| (*b).is_empty()));
170170
}
171171
}
172172

crossbeam-epoch/src/default.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
//! destructed on thread exit, which in turn unregisters the thread.
66
77
use crate::collector::{Collector, LocalHandle};
8+
use crate::concurrency::{lazy_static, thread_local};
89
use crate::guard::Guard;
9-
use lazy_static::lazy_static;
1010

1111
lazy_static! {
1212
/// The global data for the default garbage collector.
@@ -45,7 +45,7 @@ where
4545
.unwrap_or_else(|_| f(&COLLECTOR.register()))
4646
}
4747

48-
#[cfg(test)]
48+
#[cfg(all(test, not(loom)))]
4949
mod tests {
5050
use crossbeam_utils::thread;
5151

crossbeam-epoch/src/deferred.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ impl Deferred {
7676
}
7777
}
7878

79-
#[cfg(test)]
79+
#[cfg(all(test, not(loom)))]
8080
mod tests {
8181
use super::Deferred;
8282
use std::cell::Cell;

crossbeam-epoch/src/epoch.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
//! If an object became garbage in some epoch, then we can be sure that after two advancements no
88
//! participant will hold a reference to it. That is the crux of safe memory reclamation.
99
10-
use core::sync::atomic::{AtomicUsize, Ordering};
10+
use crate::concurrency::sync::atomic::AtomicUsize;
11+
use core::sync::atomic::Ordering;
1112

1213
/// An epoch that can be marked as pinned or unpinned.
1314
///

crossbeam-epoch/src/internal.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@
3535
//! Ideally each instance of concurrent data structure may have its own queue that gets fully
3636
//! destroyed as soon as the data structure gets dropped.
3737
38-
use core::cell::{Cell, UnsafeCell};
38+
use crate::concurrency::cell::UnsafeCell;
39+
use crate::concurrency::sync::atomic;
40+
use core::cell::Cell;
3941
use core::mem::{self, ManuallyDrop};
4042
use core::num::Wrapping;
41-
use core::sync::atomic;
4243
use core::sync::atomic::Ordering;
4344
use core::{fmt, ptr};
4445

@@ -408,7 +409,7 @@ impl Local {
408409
/// Returns a reference to the `Collector` in which this `Local` resides.
409410
#[inline]
410411
pub fn collector(&self) -> &Collector {
411-
unsafe { &**self.collector.get() }
412+
self.collector.with(|c| unsafe { &**c })
412413
}
413414

414415
/// Returns `true` if the current participant is pinned.
@@ -423,7 +424,7 @@ impl Local {
423424
///
424425
/// It should be safe for another thread to execute the given function.
425426
pub unsafe fn defer(&self, mut deferred: Deferred, guard: &Guard) {
426-
let bag = &mut *self.bag.get();
427+
let bag = self.bag.with_mut(|b| &mut *b);
427428

428429
while let Err(d) = bag.try_push(deferred) {
429430
let epoch = self.epoch.load(Ordering::Relaxed).unpinned();
@@ -433,7 +434,7 @@ impl Local {
433434
}
434435

435436
pub fn flush(&self, guard: &Guard) {
436-
let bag = unsafe { &mut *self.bag.get() };
437+
let bag = self.bag.with_mut(|b| unsafe { &mut *b });
437438

438439
if !bag.is_empty() {
439440
let epoch = self.epoch.load(Ordering::Relaxed).unpinned();
@@ -582,7 +583,8 @@ impl Local {
582583
// doesn't defer destruction on any new garbage.
583584
let epoch = self.epoch.load(Ordering::Relaxed).unpinned();
584585
let guard = &self.pin();
585-
self.global().push_bag(&mut *self.bag.get(), epoch, guard);
586+
self.global()
587+
.push_bag(self.bag.with_mut(|b| &mut *b), epoch, guard);
586588
}
587589
// Revert the handle count back to zero.
588590
self.handle_count.set(0);
@@ -591,7 +593,7 @@ impl Local {
591593
// Take the reference to the `Global` out of this `Local`. Since we're not protected
592594
// by a guard at this time, it's crucial that the reference is read before marking the
593595
// `Local` as deleted.
594-
let collector: Collector = ptr::read(&*(*self.collector.get()));
596+
let collector: Collector = ptr::read(self.collector.with(|c| &*(*c)));
595597

596598
// Mark this node in the linked list as deleted.
597599
self.entry.delete(&unprotected());
@@ -622,7 +624,7 @@ impl IsElement<Local> for Local {
622624
}
623625
}
624626

625-
#[cfg(test)]
627+
#[cfg(all(test, not(loom)))]
626628
mod tests {
627629
use std::sync::atomic::{AtomicUsize, Ordering};
628630

crossbeam-epoch/src/lib.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,85 @@
6060

6161
use cfg_if::cfg_if;
6262

63+
#[cfg(loom)]
64+
#[allow(unused_imports, dead_code)]
65+
pub(crate) mod concurrency {
66+
pub(crate) mod cell {
67+
pub(crate) use loom::cell::UnsafeCell;
68+
}
69+
pub(crate) mod sync {
70+
pub(crate) mod atomic {
71+
use core::sync::atomic::Ordering;
72+
pub(crate) use loom::sync::atomic::AtomicUsize;
73+
pub(crate) fn fence(ord: Ordering) {
74+
if let Ordering::Acquire = ord {
75+
} else {
76+
// FIXME: loom only supports acquire fences at the moment.
77+
// https://github.com/tokio-rs/loom/issues/117
78+
// let's at least not panic...
79+
// this may generate some false positives (`SeqCst` is stronger than `Acquire`
80+
// for example), and some false negatives (`Relaxed` is weaker than `Acquire`),
81+
// but it's the best we can do for the time being.
82+
}
83+
loom::sync::atomic::fence(Ordering::Acquire)
84+
}
85+
86+
// FIXME: loom does not support compiler_fence at the moment.
87+
// https://github.com/tokio-rs/loom/issues/117
88+
// we use fence as a stand-in for compiler_fence for the time being.
89+
// this may miss some races since fence is stronger than compiler_fence,
90+
// but it's the best we can do for the time being.
91+
pub(crate) use self::fence as compiler_fence;
92+
}
93+
pub(crate) use loom::sync::Arc;
94+
}
95+
pub(crate) use loom::lazy_static;
96+
pub(crate) use loom::thread_local;
97+
}
98+
#[cfg(not(loom))]
99+
#[allow(unused_imports, dead_code)]
100+
pub(crate) mod concurrency {
101+
#[cfg(any(feature = "alloc", feature = "std"))]
102+
pub(crate) mod cell {
103+
#[derive(Debug)]
104+
#[repr(transparent)]
105+
pub(crate) struct UnsafeCell<T>(::core::cell::UnsafeCell<T>);
106+
107+
impl<T> UnsafeCell<T> {
108+
#[inline]
109+
pub(crate) fn new(data: T) -> UnsafeCell<T> {
110+
UnsafeCell(::core::cell::UnsafeCell::new(data))
111+
}
112+
113+
#[inline]
114+
pub(crate) fn with<R>(&self, f: impl FnOnce(*const T) -> R) -> R {
115+
f(self.0.get())
116+
}
117+
118+
#[inline]
119+
pub(crate) fn with_mut<R>(&self, f: impl FnOnce(*mut T) -> R) -> R {
120+
f(self.0.get())
121+
}
122+
}
123+
}
124+
#[cfg(any(feature = "alloc", feature = "std"))]
125+
pub(crate) mod sync {
126+
pub(crate) mod atomic {
127+
pub(crate) use core::sync::atomic::compiler_fence;
128+
pub(crate) use core::sync::atomic::fence;
129+
pub(crate) use core::sync::atomic::AtomicUsize;
130+
}
131+
#[cfg_attr(feature = "nightly", cfg(target_has_atomic = "ptr"))]
132+
pub(crate) use alloc::sync::Arc;
133+
}
134+
135+
#[cfg(feature = "std")]
136+
pub(crate) use std::thread_local;
137+
138+
#[cfg(feature = "std")]
139+
pub(crate) use lazy_static::lazy_static;
140+
}
141+
63142
#[cfg_attr(feature = "nightly", cfg(target_has_atomic = "ptr"))]
64143
cfg_if! {
65144
if #[cfg(feature = "alloc")] {

0 commit comments

Comments
 (0)