From bd935570c1c4f487252209b9deb2326c0879a4b1 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 12 Jan 2022 23:09:42 +0900 Subject: [PATCH 01/12] ci: upgrade to new nightly --- .cirrus.yml | 2 +- .github/workflows/ci.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 73d77abfa1d..1f431b2d201 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -29,7 +29,7 @@ task: setup_script: - pkg install -y bash curl - curl https://sh.rustup.rs -sSf --output rustup.sh - - sh rustup.sh -y --profile minimal --default-toolchain nightly-2021-11-23 + - sh rustup.sh -y --profile minimal --default-toolchain nightly-2022-01-12 - . $HOME/.cargo/env - | echo "~~~~ rustc --version ~~~~" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0945165ae0..f9eded120da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,7 @@ name: CI env: RUSTFLAGS: -Dwarnings RUST_BACKTRACE: 1 - nightly: nightly-2021-11-23 + nightly: nightly-2022-01-12 minrust: 1.46 defaults: From 67e7d7222f6f873d546794dcdcbf8510e7ae36bb Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Thu, 13 Jan 2022 00:23:56 +0900 Subject: [PATCH 02/12] chore: test more features with Miri --- .github/workflows/ci.yml | 15 +++++++-------- tokio/src/process/unix/orphan.rs | 1 + 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f9eded120da..7811bf47a03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -166,18 +166,17 @@ jobs: - uses: actions-rs/toolchain@v1 with: toolchain: ${{ env.nightly }} + components: miri override: true - uses: Swatinem/rust-cache@v1 - - name: Install Miri - run: | - set -e - rustup component add miri - cargo miri setup - rm -rf tokio/tests - - name: miri - run: cargo miri test --features rt,rt-multi-thread,sync task + # Many of tests in tokio/tests and doctests use #[tokio::test] or + # #[tokio::main] that calls epoll_create1 that Miri does not support. + run: cargo miri test --features full --lib --no-fail-fast working-directory: tokio + env: + MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-tag-raw-pointers + san: name: san runs-on: ubuntu-latest diff --git a/tokio/src/process/unix/orphan.rs b/tokio/src/process/unix/orphan.rs index 1b0022c678e..0e52530c37b 100644 --- a/tokio/src/process/unix/orphan.rs +++ b/tokio/src/process/unix/orphan.rs @@ -280,6 +280,7 @@ pub(crate) mod test { drop(signal_guard); } + #[cfg_attr(miri, ignore)] // Miri does not support epoll. #[test] fn does_not_register_signal_if_queue_empty() { let signal_driver = IoDriver::new().and_then(SignalDriver::new).unwrap(); From 8734ac1bcbf623d19187e05dd57eb34a0dc2e2d2 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Thu, 13 Jan 2022 00:58:15 +0900 Subject: [PATCH 03/12] disable -Zmiri-tag-raw-pointers --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7811bf47a03..2c5fb059ab8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -175,7 +175,7 @@ jobs: run: cargo miri test --features full --lib --no-fail-fast working-directory: tokio env: - MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-tag-raw-pointers + MIRIFLAGS: -Zmiri-disable-isolation # -Zmiri-tag-raw-pointers san: name: san From ddcf1a3694c77fbbadc7068b383b7a632168683b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 15:56:55 +0000 Subject: [PATCH 04/12] Make tests run --- .github/workflows/ci.yml | 5 +-- tokio/src/fs/file/tests.rs | 2 ++ tokio/src/runtime/task/harness.rs | 7 +++- tokio/src/runtime/task/mod.rs | 4 +-- tokio/src/runtime/task/raw.rs | 4 +++ tokio/src/runtime/task/waker.rs | 10 +++--- tokio/src/runtime/tests/queue.rs | 20 +++++++---- tokio/src/sync/notify.rs | 5 +-- tokio/src/time/driver/mod.rs | 1 + tokio/src/util/linked_list.rs | 5 +-- tokio/src/util/slab.rs | 55 +++++++++++++++++++++++-------- 11 files changed, 85 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c5fb059ab8..440ab49bbbb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -172,10 +172,11 @@ jobs: - name: miri # Many of tests in tokio/tests and doctests use #[tokio::test] or # #[tokio::main] that calls epoll_create1 that Miri does not support. - run: cargo miri test --features full --lib --no-fail-fast + run: cargo miri test --features fs,io-util,io-std,net,parking_lot,rt,rt-multi-thread,sync --lib --no-fail-fast working-directory: tokio env: - MIRIFLAGS: -Zmiri-disable-isolation # -Zmiri-tag-raw-pointers + MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-tag-raw-pointers + PROPTEST_CASES: 10 san: name: san diff --git a/tokio/src/fs/file/tests.rs b/tokio/src/fs/file/tests.rs index 28b5ffe77af..18a4c078599 100644 --- a/tokio/src/fs/file/tests.rs +++ b/tokio/src/fs/file/tests.rs @@ -228,6 +228,7 @@ fn flush_while_idle() { } #[test] +#[cfg_attr(miri, ignore)] // takes a really long time with miri fn read_with_buffer_larger_than_max() { // Chunks let chunk_a = 16 * 1024; @@ -299,6 +300,7 @@ fn read_with_buffer_larger_than_max() { } #[test] +#[cfg_attr(miri, ignore)] // takes a really long time with miri fn write_with_buffer_larger_than_max() { // Chunks let chunk_a = 16 * 1024; diff --git a/tokio/src/runtime/task/harness.rs b/tokio/src/runtime/task/harness.rs index 0996e5232db..8a7c6b57ec2 100644 --- a/tokio/src/runtime/task/harness.rs +++ b/tokio/src/runtime/task/harness.rs @@ -26,6 +26,10 @@ where } } + fn header_ptr(&self) -> NonNull
{ + self.cell.cast() + } + fn header(&self) -> &Header { unsafe { &self.cell.as_ref().header } } @@ -93,7 +97,8 @@ where match self.header().state.transition_to_running() { TransitionToRunning::Success => { - let waker_ref = waker_ref::(self.header()); + let header_ptr = self.header_ptr(); + let waker_ref = waker_ref::(&header_ptr); let cx = Context::from_waker(&*waker_ref); let res = poll_future(&self.core().stage, cx); diff --git a/tokio/src/runtime/task/mod.rs b/tokio/src/runtime/task/mod.rs index 0592cca1a09..5075ecccc98 100644 --- a/tokio/src/runtime/task/mod.rs +++ b/tokio/src/runtime/task/mod.rs @@ -313,7 +313,7 @@ cfg_rt_multi_thread! { impl Task { fn into_raw(self) -> NonNull
{ - let ret = self.header().into(); + let ret = self.raw.header_ptr(); mem::forget(self); ret } @@ -426,7 +426,7 @@ unsafe impl linked_list::Link for Task { type Target = Header; fn as_raw(handle: &Task) -> NonNull
{ - handle.header().into() + handle.raw.header_ptr() } unsafe fn from_raw(ptr: NonNull
) -> Task { diff --git a/tokio/src/runtime/task/raw.rs b/tokio/src/runtime/task/raw.rs index fbc9574f1a4..d0b42f86247 100644 --- a/tokio/src/runtime/task/raw.rs +++ b/tokio/src/runtime/task/raw.rs @@ -57,6 +57,10 @@ impl RawTask { RawTask { ptr } } + pub(super) fn header_ptr(&self) -> NonNull
{ + self.ptr + } + /// Returns a reference to the task's meta structure. /// /// Safe as `Header` is `Sync`. diff --git a/tokio/src/runtime/task/waker.rs b/tokio/src/runtime/task/waker.rs index b7313b4c590..74a29f4a847 100644 --- a/tokio/src/runtime/task/waker.rs +++ b/tokio/src/runtime/task/waker.rs @@ -15,7 +15,7 @@ pub(super) struct WakerRef<'a, S: 'static> { /// Returns a `WakerRef` which avoids having to pre-emptively increase the /// refcount if there is no need to do so. -pub(super) fn waker_ref(header: &Header) -> WakerRef<'_, S> +pub(super) fn waker_ref(header: &NonNull
) -> WakerRef<'_, S> where T: Future, S: Schedule, @@ -28,7 +28,7 @@ where // point and not an *owned* waker, we must ensure that `drop` is never // called on this waker instance. This is done by wrapping it with // `ManuallyDrop` and then never calling drop. - let waker = unsafe { ManuallyDrop::new(Waker::from_raw(raw_waker::(header))) }; + let waker = unsafe { ManuallyDrop::new(Waker::from_raw(raw_waker::(*header))) }; WakerRef { waker, @@ -77,7 +77,7 @@ where let harness = Harness::::from_raw(ptr); trace!(harness, "waker.clone"); (*header).state.ref_inc(); - raw_waker::(header) + raw_waker::(ptr) } unsafe fn drop_waker(ptr: *const ()) @@ -114,12 +114,12 @@ where harness.wake_by_ref(); } -fn raw_waker(header: *const Header) -> RawWaker +fn raw_waker(header: NonNull
) -> RawWaker where T: Future, S: Schedule, { - let ptr = header as *const (); + let ptr = header.as_ptr() as *const (); let vtable = &RawWakerVTable::new( clone_waker::, wake_by_val::, diff --git a/tokio/src/runtime/tests/queue.rs b/tokio/src/runtime/tests/queue.rs index 47f1b01d6a6..d542be27f79 100644 --- a/tokio/src/runtime/tests/queue.rs +++ b/tokio/src/runtime/tests/queue.rs @@ -71,13 +71,21 @@ fn steal_batch() { assert!(local1.pop().is_none()); } +const fn normal_or_miri(normal: usize, miri: usize) -> usize { + if cfg!(miri) { + miri + } else { + normal + } +} + #[test] fn stress1() { const NUM_ITER: usize = 1; - const NUM_STEAL: usize = 1_000; - const NUM_LOCAL: usize = 1_000; - const NUM_PUSH: usize = 500; - const NUM_POP: usize = 250; + const NUM_STEAL: usize = normal_or_miri(1_000, 10); + const NUM_LOCAL: usize = normal_or_miri(1_000, 10); + const NUM_PUSH: usize = normal_or_miri(500, 10); + const NUM_POP: usize = normal_or_miri(250, 10); for _ in 0..NUM_ITER { let (steal, mut local) = queue::local(); @@ -133,8 +141,8 @@ fn stress1() { #[test] fn stress2() { const NUM_ITER: usize = 1; - const NUM_TASKS: usize = 1_000_000; - const NUM_STEAL: usize = 1_000; + const NUM_TASKS: usize = normal_or_miri(1_000_000, 50); + const NUM_STEAL: usize = normal_or_miri(1_000, 10); for _ in 0..NUM_ITER { let (steal, mut local) = queue::local(); diff --git a/tokio/src/sync/notify.rs b/tokio/src/sync/notify.rs index c93ce3bd45a..83d0de4fbe6 100644 --- a/tokio/src/sync/notify.rs +++ b/tokio/src/sync/notify.rs @@ -130,6 +130,7 @@ enum NotificationType { } #[derive(Debug)] +#[repr(C)] // required by `linked_list::Link` impl struct Waiter { /// Intrusive linked-list pointers. pointers: linked_list::Pointers, @@ -731,8 +732,8 @@ unsafe impl linked_list::Link for Waiter { ptr } - unsafe fn pointers(mut target: NonNull) -> NonNull> { - NonNull::from(&mut target.as_mut().pointers) + unsafe fn pointers(target: NonNull) -> NonNull> { + target.cast() } } diff --git a/tokio/src/time/driver/mod.rs b/tokio/src/time/driver/mod.rs index cf2290bc3b9..9f0b559f019 100644 --- a/tokio/src/time/driver/mod.rs +++ b/tokio/src/time/driver/mod.rs @@ -525,4 +525,5 @@ impl fmt::Debug for Inner { } #[cfg(test)] +#[cfg(not(miri))] mod tests; diff --git a/tokio/src/util/linked_list.rs b/tokio/src/util/linked_list.rs index 894d2164b9d..9356a212dc9 100644 --- a/tokio/src/util/linked_list.rs +++ b/tokio/src/util/linked_list.rs @@ -352,6 +352,7 @@ mod tests { use std::pin::Pin; #[derive(Debug)] + #[repr(C)] struct Entry { pointers: Pointers, val: i32, @@ -369,8 +370,8 @@ mod tests { Pin::new_unchecked(&*ptr.as_ptr()) } - unsafe fn pointers(mut target: NonNull) -> NonNull> { - NonNull::from(&mut target.as_mut().pointers) + unsafe fn pointers(target: NonNull) -> NonNull> { + target.cast() } } diff --git a/tokio/src/util/slab.rs b/tokio/src/util/slab.rs index 97355d500fc..45588696118 100644 --- a/tokio/src/util/slab.rs +++ b/tokio/src/util/slab.rs @@ -157,6 +157,10 @@ struct Slot { /// Next entry in the free list. next: u32, + + /// Makes miri happy. Could probably also be fixed by replacing `slots` with a raw-pointer + /// based equivalent. + _pin: std::marker::PhantomPinned, } /// Value paired with a reference to the page. @@ -409,7 +413,7 @@ impl Page { slot.value.with(|ptr| unsafe { (*ptr).value.reset() }); // Return a reference to the slot - Some((me.addr(idx), slot.gen_ref(me))) + Some((me.addr(idx), locked.gen_ref(idx, me))) } else if me.len == locked.slots.len() { // The page is full None @@ -428,9 +432,10 @@ impl Page { locked.slots.push(Slot { value: UnsafeCell::new(Value { value: Default::default(), - page: &**me as *const _, + page: Arc::as_ptr(me), }), next: 0, + _pin: std::marker::PhantomPinned, }); // Increment the head to indicate the free stack is empty @@ -443,7 +448,7 @@ impl Page { debug_assert_eq!(locked.slots.len(), locked.head); - Some((me.addr(idx), locked.slots[idx].gen_ref(me))) + Some((me.addr(idx), locked.gen_ref(idx, me))) } } } @@ -558,8 +563,21 @@ impl Slots { idx } + + /// Generates a `Ref` for the slot at the given index. This involves bumping the page's ref count. + fn gen_ref(&self, idx: usize, page: &Arc>) -> Ref { + assert!(idx < self.slots.len()); + mem::forget(page.clone()); + + let vec_ptr = self.slots.as_ptr(); + let slot: *const Slot = unsafe { vec_ptr.add(idx) }; + let value: *const Value = slot as *const Value; + + Ref { value } + } } +/* impl Slot { /// Generates a `Ref` for the slot. This involves bumping the page's ref count. fn gen_ref(&self, page: &Arc>) -> Ref { @@ -574,6 +592,7 @@ impl Slot { Ref { value } } } +*/ impl Value { /// Releases the slot, returning the `Arc>` logically owned by the ref. @@ -691,11 +710,13 @@ mod test { #[test] fn insert_many() { + const MANY: usize = normal_or_miri(10_000, 50); + let mut slab = Slab::::new(); let alloc = slab.allocator(); let mut entries = vec![]; - for i in 0..10_000 { + for i in 0..MANY { let (addr, val) = alloc.allocate().unwrap(); val.id.store(i, SeqCst); entries.push((addr, val)); @@ -708,15 +729,15 @@ mod test { entries.clear(); - for i in 0..10_000 { + for i in 0..MANY { let (addr, val) = alloc.allocate().unwrap(); - val.id.store(10_000 - i, SeqCst); + val.id.store(MANY - i, SeqCst); entries.push((addr, val)); } for (i, (addr, v)) in entries.iter().enumerate() { - assert_eq!(10_000 - i, v.id.load(SeqCst)); - assert_eq!(10_000 - i, slab.get(*addr).unwrap().id.load(SeqCst)); + assert_eq!(MANY - i, v.id.load(SeqCst)); + assert_eq!(MANY - i, slab.get(*addr).unwrap().id.load(SeqCst)); } } @@ -726,7 +747,7 @@ mod test { let alloc = slab.allocator(); let mut entries = vec![]; - for i in 0..10_000 { + for i in 0..normal_or_miri(10_000, 100) { let (addr, val) = alloc.allocate().unwrap(); val.id.store(i, SeqCst); entries.push((addr, val)); @@ -734,7 +755,7 @@ mod test { for _ in 0..10 { // Drop 1000 in reverse - for _ in 0..1_000 { + for _ in 0..normal_or_miri(1_000, 10) { entries.pop(); } @@ -753,7 +774,7 @@ mod test { let mut entries1 = vec![]; let mut entries2 = vec![]; - for i in 0..10_000 { + for i in 0..normal_or_miri(10_000, 100) { let (addr, val) = alloc.allocate().unwrap(); val.id.store(i, SeqCst); @@ -771,6 +792,14 @@ mod test { } } + const fn normal_or_miri(normal: usize, miri: usize) -> usize { + if cfg!(miri) { + miri + } else { + normal + } + } + #[test] fn compact_all() { let mut slab = Slab::::new(); @@ -780,7 +809,7 @@ mod test { for _ in 0..2 { entries.clear(); - for i in 0..10_000 { + for i in 0..normal_or_miri(10_000, 100) { let (addr, val) = alloc.allocate().unwrap(); val.id.store(i, SeqCst); @@ -808,7 +837,7 @@ mod test { let alloc = slab.allocator(); let mut entries = vec![]; - for _ in 0..5 { + for _ in 0..normal_or_miri(5, 2) { entries.clear(); // Allocate a few pages + 1 From 850600f0a7d3ea765a056e42bd5b7ee36372d767 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 16:00:17 +0000 Subject: [PATCH 05/12] Remove commented out code --- tokio/src/util/slab.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/tokio/src/util/slab.rs b/tokio/src/util/slab.rs index 45588696118..dedbec36803 100644 --- a/tokio/src/util/slab.rs +++ b/tokio/src/util/slab.rs @@ -577,23 +577,6 @@ impl Slots { } } -/* -impl Slot { - /// Generates a `Ref` for the slot. This involves bumping the page's ref count. - fn gen_ref(&self, page: &Arc>) -> Ref { - // The ref holds a ref on the page. The `Arc` is forgotten here and is - // resurrected in `release` when the `Ref` is dropped. By avoiding to - // hold on to an explicit `Arc` value, the struct size of `Ref` is - // reduced. - mem::forget(page.clone()); - let slot = self as *const Slot; - let value = slot as *const Value; - - Ref { value } - } -} -*/ - impl Value { /// Releases the slot, returning the `Arc>` logically owned by the ref. fn release(&self) -> Arc> { From 90a9e52ee06eb275f74cdf3f881dd5f4e1fcad1e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 16:42:36 +0000 Subject: [PATCH 06/12] Get Notified (and signal) to work --- .github/workflows/ci.yml | 2 +- tokio/src/signal/registry.rs | 5 ++++- tokio/src/signal/reusable_box.rs | 1 + tokio/src/sync/tests/notify.rs | 31 +++++++++++++++++++++++++++++++ tokio/src/util/wake.rs | 2 +- 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69883f90aca..a658d6706ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,7 +173,7 @@ jobs: - name: miri # Many of tests in tokio/tests and doctests use #[tokio::test] or # #[tokio::main] that calls epoll_create1 that Miri does not support. - run: cargo miri test --features fs,io-util,io-std,net,parking_lot,rt,rt-multi-thread,sync --lib --no-fail-fast + run: cargo miri test --features full --lib --no-fail-fast working-directory: tokio env: MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-tag-raw-pointers diff --git a/tokio/src/signal/registry.rs b/tokio/src/signal/registry.rs index e0a2df9208e..7f848efa3e4 100644 --- a/tokio/src/signal/registry.rs +++ b/tokio/src/signal/registry.rs @@ -202,7 +202,10 @@ mod tests { registry.broadcast(); // Yield so the previous broadcast can get received - crate::time::sleep(std::time::Duration::from_millis(10)).await; + //crate::time::sleep(std::time::Duration::from_millis(10)).await; + for _ in 0..100 { + crate::task::yield_now().await; + } // Send subsequent signal registry.record_event(0); diff --git a/tokio/src/signal/reusable_box.rs b/tokio/src/signal/reusable_box.rs index 796fa210b03..02f32474b16 100644 --- a/tokio/src/signal/reusable_box.rs +++ b/tokio/src/signal/reusable_box.rs @@ -151,6 +151,7 @@ impl fmt::Debug for ReusableBoxFuture { } #[cfg(test)] +#[cfg(not(miri))] // Miri breaks when you use Pin<&mut dyn Future> mod test { use super::ReusableBoxFuture; use futures::future::FutureExt; diff --git a/tokio/src/sync/tests/notify.rs b/tokio/src/sync/tests/notify.rs index 2828b1c342a..4ada17e791c 100644 --- a/tokio/src/sync/tests/notify.rs +++ b/tokio/src/sync/tests/notify.rs @@ -45,3 +45,34 @@ fn notify_clones_waker_before_lock() { // The result doesn't matter, we're just testing that we don't deadlock. let _ = future.poll(&mut cx); } + +#[test] +fn notify_simple() { + let notify = Notify::new(); + + let mut fut1 = tokio_test::task::spawn(notify.notified()); + assert!(fut1.poll().is_pending()); + + let mut fut2 = tokio_test::task::spawn(notify.notified()); + assert!(fut2.poll().is_pending()); + + notify.notify_waiters(); + + assert!(fut1.poll().is_ready()); + assert!(fut2.poll().is_ready()); +} + +#[test] +fn watch_test() { + let rt = crate::runtime::Builder::new_current_thread().build().unwrap(); + + rt.block_on(async { + let (tx, mut rx) = crate::sync::watch::channel(()); + + crate::spawn(async move { + let _ = tx.send(()); + }); + + let _ = rx.changed().await; + }); +} diff --git a/tokio/src/util/wake.rs b/tokio/src/util/wake.rs index 8f89668c61a..477a43da290 100644 --- a/tokio/src/util/wake.rs +++ b/tokio/src/util/wake.rs @@ -30,7 +30,7 @@ impl Deref for WakerRef<'_> { /// Creates a reference to a `Waker` from a reference to `Arc`. pub(crate) fn waker_ref(wake: &Arc) -> WakerRef<'_> { - let ptr = &**wake as *const _ as *const (); + let ptr = Arc::as_ptr(wake) as *const (); let waker = unsafe { Waker::from_raw(RawWaker::new(ptr, waker_vtable::())) }; From 939266e2089a9a1b2e476173212d5394b3e764ac Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 16:45:38 +0000 Subject: [PATCH 07/12] Disable test on wasm --- tokio/src/sync/tests/notify.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tokio/src/sync/tests/notify.rs b/tokio/src/sync/tests/notify.rs index 4ada17e791c..6cb9b302451 100644 --- a/tokio/src/sync/tests/notify.rs +++ b/tokio/src/sync/tests/notify.rs @@ -63,6 +63,7 @@ fn notify_simple() { } #[test] +#[cfg(not(target_arch = "wasm32"))] fn watch_test() { let rt = crate::runtime::Builder::new_current_thread().build().unwrap(); From 78c4bba60f7c44c7f45cf1977c25beca1d19ea8b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 16:47:06 +0000 Subject: [PATCH 08/12] rustfmt --- tokio/src/sync/tests/notify.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tokio/src/sync/tests/notify.rs b/tokio/src/sync/tests/notify.rs index 6cb9b302451..20153b7a5a8 100644 --- a/tokio/src/sync/tests/notify.rs +++ b/tokio/src/sync/tests/notify.rs @@ -65,7 +65,9 @@ fn notify_simple() { #[test] #[cfg(not(target_arch = "wasm32"))] fn watch_test() { - let rt = crate::runtime::Builder::new_current_thread().build().unwrap(); + let rt = crate::runtime::Builder::new_current_thread() + .build() + .unwrap(); rt.block_on(async { let (tx, mut rx) = crate::sync::watch::channel(()); From 45f31d318464d187b344ba1aefc8db402900122d Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 17:11:26 +0000 Subject: [PATCH 09/12] Fix time tests --- tokio/src/time/driver/entry.rs | 24 +++++++++++++----------- tokio/src/time/driver/mod.rs | 1 - tokio/src/time/driver/tests/mod.rs | 17 ++++++++++++++--- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/tokio/src/time/driver/entry.rs b/tokio/src/time/driver/entry.rs index 1beee57604b..f0ea898e120 100644 --- a/tokio/src/time/driver/entry.rs +++ b/tokio/src/time/driver/entry.rs @@ -326,15 +326,16 @@ pub(super) type EntryList = crate::util::linked_list::LinkedList, + /// Current state. This records whether the timer entry is currently under /// the ownership of the driver, and if not, its current state (not /// complete, fired, error, etc). state: StateCell, - /// Data manipulated by the driver thread itself, only. - driver_state: CachePadded, - _p: PhantomPinned, } @@ -420,7 +421,14 @@ impl TimerShared { /// padded. This contains the information that the driver thread accesses most /// frequently to minimize contention. In particular, we move it away from the /// waker, as the waker is updated on every poll. +#[repr(C)] // required by `link_list::Link` impl struct TimerSharedPadded { + /// A link within the doubly-linked list of timers on a particular level and + /// slot. Valid only if state is equal to Registered. + /// + /// Only accessed under the entry lock. + pointers: linked_list::Pointers, + /// The expiration time for which this entry is currently registered. /// Generally owned by the driver, but is accessed by the entry when not /// registered. @@ -428,12 +436,6 @@ struct TimerSharedPadded { /// The true expiration time. Set by the timer future, read by the driver. true_when: AtomicU64, - - /// A link within the doubly-linked list of timers on a particular level and - /// slot. Valid only if state is equal to Registered. - /// - /// Only accessed under the entry lock. - pointers: StdUnsafeCell>, } impl std::fmt::Debug for TimerSharedPadded { @@ -450,7 +452,7 @@ impl TimerSharedPadded { Self { cached_when: AtomicU64::new(0), true_when: AtomicU64::new(0), - pointers: StdUnsafeCell::new(linked_list::Pointers::new()), + pointers: linked_list::Pointers::new(), } } } @@ -474,7 +476,7 @@ unsafe impl linked_list::Link for TimerShared { unsafe fn pointers( target: NonNull, ) -> NonNull> { - unsafe { NonNull::new(target.as_ref().driver_state.0.pointers.get()).unwrap() } + target.cast() } } diff --git a/tokio/src/time/driver/mod.rs b/tokio/src/time/driver/mod.rs index 9f0b559f019..cf2290bc3b9 100644 --- a/tokio/src/time/driver/mod.rs +++ b/tokio/src/time/driver/mod.rs @@ -525,5 +525,4 @@ impl fmt::Debug for Inner { } #[cfg(test)] -#[cfg(not(miri))] mod tests; diff --git a/tokio/src/time/driver/tests/mod.rs b/tokio/src/time/driver/tests/mod.rs index 7c5cf1fd05c..3115285b541 100644 --- a/tokio/src/time/driver/tests/mod.rs +++ b/tokio/src/time/driver/tests/mod.rs @@ -27,7 +27,10 @@ fn block_on(f: impl std::future::Future) -> T { return loom::future::block_on(f); #[cfg(not(loom))] - return futures::executor::block_on(f); + { + let rt = crate::runtime::Builder::new_current_thread().build().unwrap(); + rt.block_on(f) + } } fn model(f: impl Fn() + Send + Sync + 'static) { @@ -182,6 +185,14 @@ fn reset_future() { }) } +fn normal_or_miri(normal: T, miri: T) -> T { + if cfg!(miri) { + miri + } else { + normal + } +} + #[test] #[cfg(not(loom))] fn poll_process_levels() { @@ -195,7 +206,7 @@ fn poll_process_levels() { let mut entries = vec![]; - for i in 0..1024 { + for i in 0..normal_or_miri(1024, 64) { let mut entry = Box::pin(TimerEntry::new( &handle, clock.now() + Duration::from_millis(i), @@ -208,7 +219,7 @@ fn poll_process_levels() { entries.push(entry); } - for t in 1..1024 { + for t in 1..normal_or_miri(1024, 64) { handle.process_at_time(t as u64); for (deadline, future) in entries.iter_mut().enumerate() { let mut context = Context::from_waker(noop_waker_ref()); From 72402b6ad94af595ef8fefb1de12f2bb3ca35d64 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 17:13:07 +0000 Subject: [PATCH 10/12] rustfmt --- tokio/src/time/driver/tests/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tokio/src/time/driver/tests/mod.rs b/tokio/src/time/driver/tests/mod.rs index 3115285b541..3ac8c756437 100644 --- a/tokio/src/time/driver/tests/mod.rs +++ b/tokio/src/time/driver/tests/mod.rs @@ -28,7 +28,9 @@ fn block_on(f: impl std::future::Future) -> T { #[cfg(not(loom))] { - let rt = crate::runtime::Builder::new_current_thread().build().unwrap(); + let rt = crate::runtime::Builder::new_current_thread() + .build() + .unwrap(); rt.block_on(f) } } @@ -185,6 +187,7 @@ fn reset_future() { }) } +#[cfg(not(loom))] fn normal_or_miri(normal: T, miri: T) -> T { if cfg!(miri) { miri From 50739c8188004f27a7f7f5761c3de76311224b1b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 28 Jan 2022 17:18:37 +0000 Subject: [PATCH 11/12] Remove comment --- tokio/src/signal/registry.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tokio/src/signal/registry.rs b/tokio/src/signal/registry.rs index 7f848efa3e4..4e4965040a6 100644 --- a/tokio/src/signal/registry.rs +++ b/tokio/src/signal/registry.rs @@ -202,7 +202,6 @@ mod tests { registry.broadcast(); // Yield so the previous broadcast can get received - //crate::time::sleep(std::time::Duration::from_millis(10)).await; for _ in 0..100 { crate::task::yield_now().await; } From 56ec538070a20d38231b8077586d45f658701322 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 2 Feb 2022 15:55:25 +0000 Subject: [PATCH 12/12] Address reviews --- tokio/src/signal/registry.rs | 3 +++ tokio/src/util/linked_list.rs | 7 +++++++ tokio/src/util/slab.rs | 4 +++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tokio/src/signal/registry.rs b/tokio/src/signal/registry.rs index 4e4965040a6..6d8eb9e7487 100644 --- a/tokio/src/signal/registry.rs +++ b/tokio/src/signal/registry.rs @@ -202,6 +202,9 @@ mod tests { registry.broadcast(); // Yield so the previous broadcast can get received + // + // This yields many times since the block_on task is only polled every 61 + // ticks. for _ in 0..100 { crate::task::yield_now().await; } diff --git a/tokio/src/util/linked_list.rs b/tokio/src/util/linked_list.rs index 6454dda3bba..db1ea7193ad 100644 --- a/tokio/src/util/linked_list.rs +++ b/tokio/src/util/linked_list.rs @@ -57,6 +57,13 @@ pub(crate) unsafe trait Link { unsafe fn from_raw(ptr: NonNull) -> Self::Handle; /// Return the pointers for a node + /// + /// # Safety + /// + /// The resulting pointer should have the same tag in the stacked-borrows + /// stack as the argument. In particular, the method may not create an + /// intermediate reference in the process of creating the resulting raw + /// pointer. unsafe fn pointers(target: NonNull) -> NonNull>; } diff --git a/tokio/src/util/slab.rs b/tokio/src/util/slab.rs index dedbec36803..214fa08dc89 100644 --- a/tokio/src/util/slab.rs +++ b/tokio/src/util/slab.rs @@ -158,7 +158,9 @@ struct Slot { /// Next entry in the free list. next: u32, - /// Makes miri happy. Could probably also be fixed by replacing `slots` with a raw-pointer + /// Makes miri happy by making mutable references not take exclusive access. + /// + /// Could probably also be fixed by replacing `slots` with a raw-pointer /// based equivalent. _pin: std::marker::PhantomPinned, }