Skip to content

Fix ci #15

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

Merged
merged 17 commits into from
Jan 18, 2022
Merged
Show file tree
Hide file tree
Changes from 16 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
27 changes: 0 additions & 27 deletions azure-pipeliens.yml

This file was deleted.

82 changes: 82 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
jobs:
- template: default.yml@templates
parameters:
minrust: 1.56.0
- template: coverage.yml@templates
parameters:
token: $(CODECOV_TOKEN_SECRET)
- job: no_std_32bit
displayName: "Compile-check on 32 bit no_std target"
pool:
vmImage: ubuntu-latest
steps:
- template: install-rust.yml@templates
parameters:
targets:
- thumbv7m-none-eabi
- bash: cargo check --target thumbv7m-none-eabi --no-default-features
displayName: cargo check
- script: |
! cargo check --target thumbv7m-none-eabi
displayName: Compilation fails with std enabled
- job: no_std_64bit
displayName: "Compile-check on 64 bit no_std target"
pool:
vmImage: ubuntu-latest
steps:
- template: install-rust.yml@templates
parameters:
targets:
- aarch64-unknown-none
- bash: cargo check --target aarch64-unknown-none --no-default-features
displayName: cargo check
- script: |
! cargo check --target aarch64-unknown-none
displayName: Compilation fails with std enabled
- job: miri
displayName: "Run miri on test suite"
pool:
vmImage: ubuntu-latest
steps:
- bash: |
echo '##vso[task.setvariable variable=nightly]nightly-'$(curl -s https://rust-lang.github.io/rustup-components-history/x86_64-unknown-linux-gnu/miri)
displayName: "Determine latest miri nightly"
- template: install-rust.yml@templates
parameters:
rust: $(nightly)
components:
- miri
- script: cargo miri test
displayName: cargo miri test
env:
MIRIFLAGS: -Zmiri-ignore-leaks
- job: loom
displayName: "Run loom on test suite"
pool:
vmImage: ubuntu-latest
steps:
- template: install-rust.yml@templates
- script: RUSTFLAGS="--cfg loom" cargo test --release --test loom
displayName: cargo test --test loom
- job: asan
displayName: "Run address sanitizer on test suite"
pool:
vmImage: ubuntu-latest
steps:
- template: install-rust.yml@templates
parameters:
rust: nightly
- bash: |
sudo ln -s /usr/bin/llvm-symbolizer-6.0 /usr/bin/llvm-symbolizer
displayName: Enable debug symbols
# only --lib --tests b/c of https://github.com/rust-lang/rust/issues/53945
- script: |
env ASAN_OPTIONS="detect_odr_violation=0:detect_leaks=0" RUSTFLAGS="-Z sanitizer=address" cargo test --lib --tests --all-features --target x86_64-unknown-linux-gnu
displayName: cargo -Z sanitizer=address test
resources:
repositories:
- repository: templates
type: github
name: crate-ci/azure-pipelines
ref: refs/heads/v0.4
endpoint: jonhoo
15 changes: 9 additions & 6 deletions src/domain.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::sync::atomic::{AtomicIsize, AtomicPtr, AtomicU64, AtomicUsize};
use crate::sync::atomic::{AtomicIsize, AtomicPtr, AtomicUsize};
use crate::{Deleter, HazPtrRecord, Reclaim};
use alloc::boxed::Box;
use alloc::collections::BTreeSet;
use core::marker::PhantomData;
use core::sync::atomic::Ordering;

#[cfg(feature = "std")]
#[cfg(all(feature = "std", not(miri)))]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow why it's necessary to turn this off under miri?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It goes with the other "time" related things so I believe it makes things cleaner when due_time is disabled (see comment below)

Copy link
Owner

@jonhoo jonhoo Jan 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I still don't understand after reading your comment. From what I can tell, miri already mocks time just fine (rust-lang/miri#975), and supports Instant::now (rust-lang/miri#1243), and I don't see anything about time-related limitations in the miri readme? What error do you get if you make miri run with time supported?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm going to merge this as-is just so we can iterate just on the miri point and have working CI in the meantime!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was getting the clock_gettime not available when isolation is enabled error, not knowing about -Zmiri-disable-isolation. This should be pretty easy to address in a PR.

const SYNC_TIME_PERIOD: u64 = std::time::Duration::from_nanos(2000000000).as_nanos() as u64;
#[cfg(all(feature = "std", not(miri)))]
use crate::sync::atomic::AtomicU64;

#[cfg(loom)]
const RCOUNT_THRESHOLD: isize = 5;
Expand Down Expand Up @@ -53,6 +55,7 @@ pub struct Domain<F> {
hazptrs: HazPtrRecords,
untagged: [RetiredList; NUM_SHARDS],
family: PhantomData<F>,
#[cfg(all(feature = "std", not(miri)))]
due_time: AtomicU64,
nbulk_reclaims: AtomicUsize,
count: AtomicIsize,
Expand Down Expand Up @@ -101,6 +104,7 @@ macro_rules! new {
},
untagged,
count: AtomicIsize::new(0),
#[cfg(all(feature = "std", not(miri)))]
due_time: AtomicU64::new(0),
nbulk_reclaims: AtomicUsize::new(0),
family: PhantomData,
Expand Down Expand Up @@ -357,9 +361,8 @@ impl<F> Domain<F> {
.compare_exchange_weak(rcount, 0, Ordering::AcqRel, Ordering::Relaxed)
{
Ok(_) => {
#[cfg(feature = "std")]
#[cfg(all(feature = "std", not(miri)))]
{
// We don't check `due_time` with no_std, so no need to set it either
self.due_time
.store(Self::now() + SYNC_TIME_PERIOD, Ordering::Release);
}
Expand All @@ -371,7 +374,7 @@ impl<F> Domain<F> {
0
}

#[cfg(feature = "std")]
#[cfg(all(feature = "std", not(miri)))]
fn check_due_time(&self) -> isize {
let time = Self::now();
let due = self.due_time.load(Ordering::Acquire);
Expand Down Expand Up @@ -399,7 +402,7 @@ impl<F> Domain<F> {
// TODO: Implement some kind of mock time for no_std.
// Currently we reclaim only based on rcount on no_std
// (also the reason for allow unused_mut)
#[cfg(feature = "std")]
#[cfg(all(feature = "std", not(miri)))]
{
rcount = self.check_due_time();
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(arbitrary_self_types)]
//#![feature(arbitrary_self_types)]
#![deny(unsafe_op_in_unsafe_fn)]
#![allow(dead_code)]
#![cfg_attr(not(feature = "std"), no_std)]
Expand Down
6 changes: 3 additions & 3 deletions src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ where
///
/// It is okay for existing readers to still refer to Self.
///
unsafe fn retire(self: *mut Self, deleter: &'static dyn Deleter) -> usize {
let ptr = self as *mut (dyn Reclaim + 'domain);
unsafe { (&*self).domain().retire(ptr, deleter) }
unsafe fn retire(this: *mut Self, deleter: &'static dyn Deleter) -> usize {
let ptr = this as *mut (dyn Reclaim + 'domain);
unsafe { (&*this).domain().retire(ptr, deleter) }
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ pub(crate) use loom::thread::yield_now;

#[cfg(not(loom))]
pub(crate) mod atomic {
pub(crate) use core::sync::atomic::{fence, AtomicIsize, AtomicPtr, AtomicU64, AtomicUsize};
pub(crate) use core::sync::atomic::{fence, AtomicIsize, AtomicPtr, AtomicUsize};
#[cfg(target_pointer_width = "64")]
pub use core::sync::atomic::AtomicU64;
}
#[cfg(all(not(loom), feature = "std"))]
pub(crate) use std::thread::yield_now;
8 changes: 4 additions & 4 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ fn acquires_multiple() {
domain.eager_reclaim();
assert_eq!(drops_42.load(Ordering::SeqCst), 0);

unsafe { x.into_inner().retire(&deleters::drop_box) };
unsafe { HazPtrObjectWrapper::retire(x.into_inner(), &deleters::drop_box) };
domain.eager_reclaim();
assert_eq!(drops_42.load(Ordering::SeqCst), 1);

unsafe { y.into_inner().retire(&deleters::drop_box) };
unsafe { HazPtrObjectWrapper::retire(y.into_inner(), &deleters::drop_box) };
domain.eager_reclaim();
assert_eq!(drops_42.load(Ordering::SeqCst), 2);
}
Expand Down Expand Up @@ -130,7 +130,7 @@ fn feels_good() {
// 1. The pointer came from Box, so is valid.
// 2. The old value is no longer accessible.
// 3. The deleter is valid for Box types.
unsafe { old.retire(&deleters::drop_box) };
unsafe { HazPtrObjectWrapper::retire(old, &deleters::drop_box) };

assert_eq!(drops_42.load(Ordering::SeqCst), 0);
assert_eq!(my_x.0, 42);
Expand Down Expand Up @@ -207,7 +207,7 @@ fn drop_domain() {
assert_eq!(drops_42.load(Ordering::SeqCst), 0);
assert_eq!(my_x.0, 42);

unsafe { old.retire(&deleters::drop_box) };
unsafe { HazPtrObjectWrapper::retire(old, &deleters::drop_box) };

assert_eq!(drops_42.load(Ordering::SeqCst), 0);
assert_eq!(my_x.0, 42);
Expand Down
8 changes: 4 additions & 4 deletions tests/loom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,12 @@ fn acquires_multiple() {
assert_eq!(ndrops_1.load(Ordering::SeqCst), 0);
assert_eq!(ndrops_2.load(Ordering::SeqCst), 0);

unsafe { x.load(Ordering::SeqCst).retire(&deleters::drop_box) };
unsafe { HazPtrObjectWrapper::retire(x.load(Ordering::SeqCst), &deleters::drop_box) };
domain.eager_reclaim();
assert_eq!(ndrops_1.load(Ordering::SeqCst), 1);
assert_eq!(ndrops_2.load(Ordering::SeqCst), 0);

unsafe { y.load(Ordering::SeqCst).retire(&deleters::drop_box) };
unsafe { HazPtrObjectWrapper::retire(y.load(Ordering::SeqCst), &deleters::drop_box) };
domain.eager_reclaim();
assert_eq!(ndrops_1.load(Ordering::SeqCst), 1);
assert_eq!(ndrops_2.load(Ordering::SeqCst), 1);
Expand Down Expand Up @@ -179,7 +179,7 @@ fn single_reader_protection() {
)))),
std::sync::atomic::Ordering::SeqCst,
);
let n0 = unsafe { old.retire(&deleters::drop_box) };
let n0 = unsafe { HazPtrObjectWrapper::retire(old, &deleters::drop_box) };

let n1 = Domain::global().eager_reclaim();

Expand Down Expand Up @@ -244,7 +244,7 @@ fn multi_reader_protection() {
)))),
std::sync::atomic::Ordering::SeqCst,
);
let n0 = unsafe { old.retire(&deleters::drop_box) };
let n0 = unsafe { HazPtrObjectWrapper::retire(old, &deleters::drop_box) };

let n1 = Domain::global().eager_reclaim();

Expand Down